On 02/19/16 19:13, Wei Huang wrote: > The condition checking on vrng->conf.period_ms appears to be wrong, > conflicting with the error comment following it. > > Signed-off-by: Wei Huang <w...@redhat.com> > --- > hw/virtio/virtio-rng.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c > index 473c044..a06427c 100644 > --- a/hw/virtio/virtio-rng.c > +++ b/hw/virtio/virtio-rng.c > @@ -149,7 +149,7 @@ static void virtio_rng_device_realize(DeviceState *dev, > Error **errp) > VirtIORNG *vrng = VIRTIO_RNG(dev); > Error *local_err = NULL; > > - if (!vrng->conf.period_ms > 0) { > + if (!(vrng->conf.period_ms > 0)) { > error_setg(errp, "'period' parameter expects a positive integer"); > return; > } >
The current condition is absolutely weird, but I think it happens to work correctly: Period_ms has type uint32_t. If it is positive, then !period_ms is zero. 0>0 is false, hence the error message is not printed. If period_ms is zero, then !period_ms is 1. 1>0 is true, hence the error message is printed. I would rewrite the check as if (vrng->conf.period_ms == 0) { error_setg(...) Thanks Laszlo