From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:36977) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TB8ve-0005nZ-8C for qemu-devel@nongnu.org; Mon, 10 Sep 2012 14:41:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TB8vZ-0004Ut-TG for qemu-devel@nongnu.org; Mon, 10 Sep 2012 14:41:50 -0400 Received: from thoth.sbs.de ([192.35.17.2]:30750) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TB8vZ-0004Ul-JW for qemu-devel@nongnu.org; Mon, 10 Sep 2012 14:41:45 -0400 Message-ID: <504E3467.7040408@siemens.com> Date: Mon, 10 Sep 2012 20:41:43 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1347300346-14482-1-git-send-email-hpoussin@reactos.org> <1347300346-14482-3-git-send-email-hpoussin@reactos.org> In-Reply-To: <1347300346-14482-3-git-send-email-hpoussin@reactos.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 2/3] slirp: Handle more than 65535 blocks in TFTP transfers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?SGVydsOpIFBvdXNzaW5lYXU=?= Cc: "qemu-devel@nongnu.org" On 2012-09-10 20:05, Herv=C3=A9 Poussineau wrote: > RFC 1350 does not mention block count roll-over. However, a lot of TFTP= servers > implement it to be able to transmit big files, so do it also. >=20 > Current block size is 512 bytes, so TFTP files were limited to 32 MB. >=20 > Signed-off-by: Herv=C3=A9 Poussineau > Reviewed-by: Aurelien Jarno > --- > slirp/tftp.c | 9 +++++---- > slirp/tftp.h | 1 + > 2 files changed, 6 insertions(+), 4 deletions(-) >=20 > diff --git a/slirp/tftp.c b/slirp/tftp.c > index 520dbd6..75c9030 100644 > --- a/slirp/tftp.c > +++ b/slirp/tftp.c > @@ -97,7 +97,7 @@ static int tftp_session_find(Slirp *slirp, struct tft= p_t *tp) > return -1; > } > =20 > -static int tftp_read_data(struct tftp_session *spt, uint16_t block_nr, > +static int tftp_read_data(struct tftp_session *spt, uint32_t block_nr, > uint8_t *buf, int len) > { > int bytes_read =3D 0; > @@ -198,7 +198,7 @@ out: > } > =20 > static int tftp_send_data(struct tftp_session *spt, > - uint16_t block_nr, > + uint32_t block_nr, > struct tftp_t *recv_tp) > { > struct sockaddr_in saddr, daddr; > @@ -223,7 +223,7 @@ static int tftp_send_data(struct tftp_session *spt, > m->m_data +=3D sizeof(struct udpiphdr); > =20 > tp->tp_op =3D htons(TFTP_DATA); > - tp->x.tp_data.tp_block_nr =3D htons(block_nr); > + tp->x.tp_data.tp_block_nr =3D htons(block_nr & 0xffff); > =20 > saddr.sin_addr =3D recv_tp->ip.ip_dst; > saddr.sin_port =3D recv_tp->udp.uh_dport; > @@ -255,6 +255,7 @@ static int tftp_send_data(struct tftp_session *spt, > tftp_session_terminate(spt); > } > =20 > + spt->block_nr =3D block_nr; That's not really a nice interface: You pass in tftp_session::block_nr as argument but you also manipulate it here. I would vote for some pure variant: either implement a void tftp_send_next_block(struct tftp_session *spt, struct tftp_t *recv_tp) , initializing tftp_session::block_nr to 0 before first call and incrementing it inside on success. Or do the maintenance of block_nr completely outside. I leaning a bit to variant 1. > return 0; > } > =20 > @@ -387,7 +388,7 @@ static void tftp_handle_ack(Slirp *slirp, struct tf= tp_t *tp, int pktlen) > } > =20 > if (tftp_send_data(&slirp->tftp_sessions[s], > - ntohs(tp->x.tp_data.tp_block_nr) + 1, > + slirp->tftp_sessions[s].block_nr + 1, > tp) < 0) { > return; > } > diff --git a/slirp/tftp.h b/slirp/tftp.h > index 9c364ea..51704e4 100644 > --- a/slirp/tftp.h > +++ b/slirp/tftp.h > @@ -37,6 +37,7 @@ struct tftp_session { > =20 > struct in_addr client_ip; > uint16_t client_port; > + uint32_t block_nr; > =20 > int timestamp; > }; >=20 Jan --=20 Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux