Re: [Xen-devel] [PATCH] Mini-OS: netfront: fix off-by-one error introduced in 7c8f3483

[Samuel Thibault]

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

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 <s...@prgmr.com>
Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org>

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;
 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel