From mboxrd@z Thu Jan 1 00:00:00 1970 From: Samuel Thibault Subject: Re: [PATCH] Mini-OS: netfront: fix off-by-one error introduced in 7c8f3483 Date: Fri, 25 Mar 2016 19:33:20 +0100 Message-ID: <20160325183320.GB2792@var> References: <1458768411-6024-1-git-send-email-srn@prgmr.com> <20160325130907.GC20441@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="ZPt4rx8FFjLCG7dd" Return-path: Content-Disposition: inline In-Reply-To: <20160325130907.GC20441@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Wei Liu Cc: xen-devel@lists.xen.org, minios-devel@lists.xen.org, Sarah Newman List-Id: xen-devel@lists.xenproject.org --ZPt4rx8FFjLCG7dd Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Wei Liu, on Fri 25 Mar 2016 13:09:07 +0000, wrote: > CC Samuel Thanks. > On Wed, Mar 23, 2016 at 02:26:51PM -0700, Sarah Newman wrote: > > 7c8f3483 introduced a break within a loop in netfront.c such that > > cons and nr_consumed were no longer always being incremented. The > > offset at cons will be processed multiple times with the break in > > place. > > > > Remove the break and re-add "some !=0" in the loop for HAVE_LIBC. Mmm, right. That ifdef makes things even more difficult to understand then. That however makes me think: how about the attached patch, which actually simplifies the rest. Thanks! Samuel --ZPt4rx8FFjLCG7dd Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=patch netfront: fix off-by-one error introduced in 7c8f3483 7c8f3483 introduced a break within a loop in netfront.c such that cons and nr_consumed were no longer always being incremented. The offset at cons will be processed multiple times with the break in place. Remove the break and re-add "some !=0" in the loop for HAVE_LIBC, rename it into dobreak to mitigate confusion. Signed-off-by: Sarah Newman Signed-off-by: Samuel Thibault diff --git a/netfront.c b/netfront.c index 0eca5b5..b8fac62 100644 --- a/netfront.c +++ b/netfront.c @@ -97,19 +97,15 @@ void network_rx(struct netfront_dev *dev) { RING_IDX rp,cons,req_prod; int nr_consumed, more, i, notify; -#ifdef HAVE_LIBC - int some; -#endif + int dobreak; nr_consumed = 0; moretodo: rp = dev->rx.sring->rsp_prod; rmb(); /* Ensure we see queued responses up to 'rp'. */ -#ifdef HAVE_LIBC - some = 0; -#endif - for (cons = dev->rx.rsp_cons; cons != rp; nr_consumed++, cons++) + dobreak = 0; + for (cons = dev->rx.rsp_cons; cons != rp && !dobreak; nr_consumed++, cons++) { struct net_buffer* buf; unsigned char* page; @@ -134,8 +130,8 @@ moretodo: len = dev->len; memcpy(dev->data, page+rx->offset, len); dev->rlen = len; - some = 1; - break; + /* No need to receive the rest for now */ + dobreak = 1; } else #endif dev->netif_rx(page+rx->offset,rx->status); @@ -144,11 +140,7 @@ moretodo: dev->rx.rsp_cons=cons; RING_FINAL_CHECK_FOR_RESPONSES(&dev->rx,more); -#ifdef HAVE_LIBC - if(more && !some) goto moretodo; -#else - if(more) goto moretodo; -#endif + if(more && !dobreak) goto moretodo; req_prod = dev->rx.req_prod_pvt; --ZPt4rx8FFjLCG7dd Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --ZPt4rx8FFjLCG7dd--