| Message ID | 1320872963-10402-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Commit | e7852674d5013bffd0bb8e822a7521f76677a60b |
| Headers | show |
On 9 November 2011 22:09, Peter Maydell <peter.maydell@linaro.org> wrote: > "!X == 2" is always false (spotted by Coverity), so the checks > for whether rndis is in the correct state would never fire. I pushed this patch because it's a bugfix and the check is guarded by is_rndis() so there should be no risk of affecting non-rndis. Cheers
On 14 November 2011 03:00, andrzej zaborowski <balrogg@gmail.com> wrote: > On 9 November 2011 22:09, Peter Maydell <peter.maydell@linaro.org> wrote: >> "!X == 2" is always false (spotted by Coverity), so the checks >> for whether rndis is in the correct state would never fire. > > I pushed this patch because it's a bugfix and the check is guarded by > is_rndis() so there should be no risk of affecting non-rndis. I'm happy that non-rndis works, I tested that. What I don't know is whether the patch breaks rndis -- it's possible the checks were wrong all along but we never noticed because of this bug. That's why I suggested that somebody should test the rndis case before applying... -- PMM
On 14 November 2011 09:08, Peter Maydell <peter.maydell@linaro.org> wrote: > I'm happy that non-rndis works, I tested that. What I don't know > is whether the patch breaks rndis Sorry, I misread what you said assuming that you tested a branch affected by this patch. I'm unable to find a guest to test the rndis mode so I reverted the patch until after release. Applying is probably the best way to get someone to test a change like this. Cheers
On 14 November 2011 17:21, andrzej zaborowski <balrogg@gmail.com> wrote: > On 14 November 2011 09:08, Peter Maydell <peter.maydell@linaro.org> wrote: >> I'm happy that non-rndis works, I tested that. What I don't know >> is whether the patch breaks rndis > > Sorry, I misread what you said assuming that you tested a branch > affected by this patch. I'm unable to find a guest to test the rndis > mode so I reverted the patch until after release. Applying is > probably the best way to get someone to test a change like this. Well, we're past release now, I think we could reasonably (re)apply this patch now? thanks -- PMM
On 11/09/2011 03:09 PM, Peter Maydell wrote: > "!X == 2" is always false (spotted by Coverity), so the checks > for whether rndis is in the correct state would never fire. > > Signed-off-by: Peter Maydell<peter.maydell@linaro.org> Applied. Thanks. Regards, Anthony Liguori > --- > NB that although I tested that this doesn't break non-rndis > usb-net, I don't have a test image that uses rndis usb-net, > so treat this patch with the appropriate degree of caution. > (Probably safer not putting it into 1.0 unless tested.) > > hw/usb-net.c | 5 +++-- > 1 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/hw/usb-net.c b/hw/usb-net.c > index a8b7c8d..f91fa32 100644 > --- a/hw/usb-net.c > +++ b/hw/usb-net.c > @@ -1268,8 +1268,9 @@ static ssize_t usbnet_receive(VLANClientState *nc, const uint8_t *buf, size_t si > > if (is_rndis(s)) { > msg = (struct rndis_packet_msg_type *) s->in_buf; > - if (!s->rndis_state == RNDIS_DATA_INITIALIZED) > + if (s->rndis_state != RNDIS_DATA_INITIALIZED) { > return -1; > + } > if (size + sizeof(struct rndis_packet_msg_type)> sizeof(s->in_buf)) > return -1; > > @@ -1302,7 +1303,7 @@ static int usbnet_can_receive(VLANClientState *nc) > { > USBNetState *s = DO_UPCAST(NICState, nc, nc)->opaque; > > - if (is_rndis(s)&& !s->rndis_state == RNDIS_DATA_INITIALIZED) { > + if (is_rndis(s)&& s->rndis_state != RNDIS_DATA_INITIALIZED) { > return 1; > } >
diff --git a/hw/usb-net.c b/hw/usb-net.c index a8b7c8d..f91fa32 100644 --- a/hw/usb-net.c +++ b/hw/usb-net.c @@ -1268,8 +1268,9 @@ static ssize_t usbnet_receive(VLANClientState *nc, const uint8_t *buf, size_t si if (is_rndis(s)) { msg = (struct rndis_packet_msg_type *) s->in_buf; - if (!s->rndis_state == RNDIS_DATA_INITIALIZED) + if (s->rndis_state != RNDIS_DATA_INITIALIZED) { return -1; + } if (size + sizeof(struct rndis_packet_msg_type) > sizeof(s->in_buf)) return -1; @@ -1302,7 +1303,7 @@ static int usbnet_can_receive(VLANClientState *nc) { USBNetState *s = DO_UPCAST(NICState, nc, nc)->opaque; - if (is_rndis(s) && !s->rndis_state == RNDIS_DATA_INITIALIZED) { + if (is_rndis(s) && s->rndis_state != RNDIS_DATA_INITIALIZED) { return 1; }
"!X == 2" is always false (spotted by Coverity), so the checks for whether rndis is in the correct state would never fire. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- NB that although I tested that this doesn't break non-rndis usb-net, I don't have a test image that uses rndis usb-net, so treat this patch with the appropriate degree of caution. (Probably safer not putting it into 1.0 unless tested.) hw/usb-net.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)