xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Mini-OS: netfront: fix off-by-one error introduced in 7c8f3483
@ 2016-04-01 18:26 Samuel Thibault
  2016-04-04 10:28 ` Wei Liu
  0 siblings, 1 reply; 13+ messages in thread
From: Samuel Thibault @ 2016-04-01 18:26 UTC (permalink / raw)
  To: xen-devel, minios-devel; +Cc: Samuel Thibault, wei.liu2, srn

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.

This commit reverts to using the "some" variable in the loop condition,
but avoids ifdefs for the non-libc case. It also renames it to dobreak
to make its usage clearer.

Suggested-by: Sarah Newman <srn@prgmr.com>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Tested-by: Sarah Newman <srn@prgmr.com>
---
 netfront.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

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;
 
-- 
2.8.0.rc3


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

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH] Mini-OS: netfront: fix off-by-one error introduced in 7c8f3483
@ 2016-04-01 18:17 Samuel Thibault
  2016-04-01 18:26 ` Samuel Thibault
  0 siblings, 1 reply; 13+ messages in thread
From: Samuel Thibault @ 2016-04-01 18:17 UTC (permalink / raw)
  To: xen-devel, minios-devel; +Cc: Samuel Thibault, wei.liu2, srn

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.

This commit reverts to using the "some" variable in the loop condition,
but avoids ifdefs for the non-libc case. It also renames it to dobreak
to make its usage clearer.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Tested-by: Sarah Newman <srn@prgmr.com>
---
 netfront.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

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;
 
-- 
2.8.0.rc3


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

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH] Mini-OS: netfront: fix off-by-one error introduced in 7c8f3483
@ 2016-03-23 21:26 Sarah Newman
  2016-03-25 13:09 ` Wei Liu
  0 siblings, 1 reply; 13+ messages in thread
From: Sarah Newman @ 2016-03-23 21:26 UTC (permalink / raw)
  To: xen-devel; +Cc: minios-devel, Sarah Newman

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.

Signed-off-by: Sarah Newman <srn@prgmr.com>
---
 netfront.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/netfront.c b/netfront.c
index 0eca5b5..557e8c4 100644
--- a/netfront.c
+++ b/netfront.c
@@ -108,8 +108,10 @@ moretodo:
 
 #ifdef HAVE_LIBC
     some = 0;
-#endif
+    for (cons = dev->rx.rsp_cons; (cons != rp) && !some; nr_consumed++, cons++)
+#else
     for (cons = dev->rx.rsp_cons; cons != rp; nr_consumed++, cons++)
+#endif
     {
         struct net_buffer* buf;
         unsigned char* page;
@@ -135,7 +137,6 @@ moretodo:
 		memcpy(dev->data, page+rx->offset, len);
 		dev->rlen = len;
 		some = 1;
-                break;
 	    } else
 #endif
 		dev->netif_rx(page+rx->offset,rx->status);
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2016-04-25 11:14 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-01 18:26 [PATCH] Mini-OS: netfront: fix off-by-one error introduced in 7c8f3483 Samuel Thibault
2016-04-04 10:28 ` Wei Liu
2016-04-25 11:14   ` Wei Liu
  -- strict thread matches above, loose matches on Subject: below --
2016-04-01 18:17 Samuel Thibault
2016-04-01 18:26 ` Samuel Thibault
2016-03-23 21:26 Sarah Newman
2016-03-25 13:09 ` Wei Liu
2016-03-25 18:33   ` Samuel Thibault
2016-03-25 19:19     ` Sarah Newman
2016-03-25 19:32       ` Samuel Thibault
2016-03-25 19:56         ` Sarah Newman
2016-03-26 20:53     ` Sarah Newman
2016-03-30 13:46       ` Wei Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).