| Message ID | 20170602081519.15648-1-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
* Marc-André Lureau (marcandre.lureau@redhat.com) wrote: > CC tests/vhost-user-bridge.o > /home/dgilbert/git/qemu-world3/tests/vhost-user-bridge.c:228:23: warning: variables 'front' and 'iov' used in loop condition not modified in loop body [-Wfor-loop-analysis] > for (cur = front; front != iov; cur++) { > ^~~~~ ~~~ > 1 warning generated. > > Fix the loop, document the function, and fix some related assert(). > > In practice, the loop bug was harmless because the front sg buffer is > enough to discard/restore the header size. > > Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > tests/vhost-user-bridge.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c > index 8618c20d53..1e5b5ca3da 100644 > --- a/tests/vhost-user-bridge.c > +++ b/tests/vhost-user-bridge.c > @@ -220,12 +220,18 @@ vubr_handle_tx(VuDev *dev, int qidx) > free(elem); > } > > + > +/* this function reverse the effect of iov_discard_front() it must be > + * called with 'front' being the original struct iovec and 'bytes' > + * being the number of bytes you shaved off > + */ > static void > iov_restore_front(struct iovec *front, struct iovec *iov, size_t bytes) > { > struct iovec *cur; > > - for (cur = front; front != iov; cur++) { > + for (cur = front; cur != iov; cur++) { > + assert(bytes >= cur->iov_len); > bytes -= cur->iov_len; > } > > @@ -302,7 +308,8 @@ vubr_backend_recv_cb(int sock, void *ctx) > } > iov_from_buf(sg, elem->in_num, 0, &hdr, sizeof hdr); > total += hdrlen; > - assert(iov_discard_front(&sg, &num, hdrlen) == hdrlen); > + ret = iov_discard_front(&sg, &num, hdrlen); > + assert(ret == hdrlen); > } > > struct msghdr msg = { > -- > 2.13.0.91.g00982b8dd > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Fri, Jun 02, 2017 at 12:15:19PM +0400, Marc-André Lureau wrote: > CC tests/vhost-user-bridge.o > /home/dgilbert/git/qemu-world3/tests/vhost-user-bridge.c:228:23: warning: variables 'front' and 'iov' used in loop condition not modified in loop body [-Wfor-loop-analysis] > for (cur = front; front != iov; cur++) { > ^~~~~ ~~~ > 1 warning generated. > > Fix the loop, document the function, and fix some related assert(). > > In practice, the loop bug was harmless because the front sg buffer is > enough to discard/restore the header size. > > Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > tests/vhost-user-bridge.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c > index 8618c20d53..1e5b5ca3da 100644 > --- a/tests/vhost-user-bridge.c > +++ b/tests/vhost-user-bridge.c > @@ -220,12 +220,18 @@ vubr_handle_tx(VuDev *dev, int qidx) > free(elem); > } > > + > +/* this function reverse the effect of iov_discard_front() it must be > + * called with 'front' being the original struct iovec and 'bytes' > + * being the number of bytes you shaved off > + */ > static void > iov_restore_front(struct iovec *front, struct iovec *iov, size_t bytes) > { > struct iovec *cur; > > - for (cur = front; front != iov; cur++) { > + for (cur = front; cur != iov; cur++) { > + assert(bytes >= cur->iov_len); > bytes -= cur->iov_len; This fixes a problem I saw when trying to pxe-boot with vhost-user and vhost-user-bridge. It was running in an endless loop here. Fixed with this patch. Tested-by: Jens Freimann <jfreiman@redhat.com> regards, Jens
Hi, Michael, could you pick this patch? thanks On Fri, Jun 2, 2017 at 12:26 PM Marc-André Lureau < marcandre.lureau@redhat.com> wrote: > CC tests/vhost-user-bridge.o > /home/dgilbert/git/qemu-world3/tests/vhost-user-bridge.c:228:23: warning: > variables 'front' and 'iov' used in loop condition not modified in loop > body [-Wfor-loop-analysis] > for (cur = front; front != iov; cur++) { > ^~~~~ ~~~ > 1 warning generated. > > Fix the loop, document the function, and fix some related assert(). > > In practice, the loop bug was harmless because the front sg buffer is > enough to discard/restore the header size. > > Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > tests/vhost-user-bridge.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c > index 8618c20d53..1e5b5ca3da 100644 > --- a/tests/vhost-user-bridge.c > +++ b/tests/vhost-user-bridge.c > @@ -220,12 +220,18 @@ vubr_handle_tx(VuDev *dev, int qidx) > free(elem); > } > > + > +/* this function reverse the effect of iov_discard_front() it must be > + * called with 'front' being the original struct iovec and 'bytes' > + * being the number of bytes you shaved off > + */ > static void > iov_restore_front(struct iovec *front, struct iovec *iov, size_t bytes) > { > struct iovec *cur; > > - for (cur = front; front != iov; cur++) { > + for (cur = front; cur != iov; cur++) { > + assert(bytes >= cur->iov_len); > bytes -= cur->iov_len; > } > > @@ -302,7 +308,8 @@ vubr_backend_recv_cb(int sock, void *ctx) > } > iov_from_buf(sg, elem->in_num, 0, &hdr, sizeof hdr); > total += hdrlen; > - assert(iov_discard_front(&sg, &num, hdrlen) == hdrlen); > + ret = iov_discard_front(&sg, &num, hdrlen); > + assert(ret == hdrlen); > } > > struct msghdr msg = { > -- > 2.13.0.91.g00982b8dd > > > -- Marc-André Lureau
On Wed, Jun 07, 2017 at 07:00:46PM +0000, Marc-André Lureau wrote: > Hi, Michael, could you pick this patch? > > thanks OK. Pls cc me on patches you want me to review. > On Fri, Jun 2, 2017 at 12:26 PM Marc-André Lureau < > marcandre.lureau@redhat.com> wrote: > > > CC tests/vhost-user-bridge.o > > /home/dgilbert/git/qemu-world3/tests/vhost-user-bridge.c:228:23: warning: > > variables 'front' and 'iov' used in loop condition not modified in loop > > body [-Wfor-loop-analysis] > > for (cur = front; front != iov; cur++) { > > ^~~~~ ~~~ > > 1 warning generated. > > > > Fix the loop, document the function, and fix some related assert(). > > > > In practice, the loop bug was harmless because the front sg buffer is > > enough to discard/restore the header size. > > > > Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > --- > > tests/vhost-user-bridge.c | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c > > index 8618c20d53..1e5b5ca3da 100644 > > --- a/tests/vhost-user-bridge.c > > +++ b/tests/vhost-user-bridge.c > > @@ -220,12 +220,18 @@ vubr_handle_tx(VuDev *dev, int qidx) > > free(elem); > > } > > > > + > > +/* this function reverse the effect of iov_discard_front() it must be > > + * called with 'front' being the original struct iovec and 'bytes' > > + * being the number of bytes you shaved off > > + */ > > static void > > iov_restore_front(struct iovec *front, struct iovec *iov, size_t bytes) > > { > > struct iovec *cur; > > > > - for (cur = front; front != iov; cur++) { > > + for (cur = front; cur != iov; cur++) { > > + assert(bytes >= cur->iov_len); > > bytes -= cur->iov_len; > > } > > > > @@ -302,7 +308,8 @@ vubr_backend_recv_cb(int sock, void *ctx) > > } > > iov_from_buf(sg, elem->in_num, 0, &hdr, sizeof hdr); > > total += hdrlen; > > - assert(iov_discard_front(&sg, &num, hdrlen) == hdrlen); > > + ret = iov_discard_front(&sg, &num, hdrlen); > > + assert(ret == hdrlen); > > } > > > > struct msghdr msg = { > > -- > > 2.13.0.91.g00982b8dd > > > > > > -- > Marc-André Lureau
diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c index 8618c20d53..1e5b5ca3da 100644 --- a/tests/vhost-user-bridge.c +++ b/tests/vhost-user-bridge.c @@ -220,12 +220,18 @@ vubr_handle_tx(VuDev *dev, int qidx) free(elem); } + +/* this function reverse the effect of iov_discard_front() it must be + * called with 'front' being the original struct iovec and 'bytes' + * being the number of bytes you shaved off + */ static void iov_restore_front(struct iovec *front, struct iovec *iov, size_t bytes) { struct iovec *cur; - for (cur = front; front != iov; cur++) { + for (cur = front; cur != iov; cur++) { + assert(bytes >= cur->iov_len); bytes -= cur->iov_len; } @@ -302,7 +308,8 @@ vubr_backend_recv_cb(int sock, void *ctx) } iov_from_buf(sg, elem->in_num, 0, &hdr, sizeof hdr); total += hdrlen; - assert(iov_discard_front(&sg, &num, hdrlen) == hdrlen); + ret = iov_discard_front(&sg, &num, hdrlen); + assert(ret == hdrlen); } struct msghdr msg = {
CC tests/vhost-user-bridge.o /home/dgilbert/git/qemu-world3/tests/vhost-user-bridge.c:228:23: warning: variables 'front' and 'iov' used in loop condition not modified in loop body [-Wfor-loop-analysis] for (cur = front; front != iov; cur++) { ^~~~~ ~~~ 1 warning generated. Fix the loop, document the function, and fix some related assert(). In practice, the loop bug was harmless because the front sg buffer is enough to discard/restore the header size. Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- tests/vhost-user-bridge.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)