| Message ID | 1424448376-15599-2-git-send-email-rkrcmar@redhat.com |
|---|---|
| State | New |
| Headers | show |
20.02.2015 19:06, Radim Krčmář wrote: [] > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 45da34ad6129..93818675588e 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -120,8 +120,8 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status) > return; > } > > - if (!!n->vhost_started == > - (virtio_net_started(n, status) && !nc->peer->link_down)) { > + if ((virtio_net_started(n, status) && !nc->peer->link_down) == > + !!n->vhost_started) { btw, can this be rewritten as (bool)n->vhost_started instead of !!n->vhos_started ? Not questioning the patch itself, just wondering, as these double-negatives look ugly... Thanks, /mjt
2015-03-04 17:36+0300, Michael Tokarev: > 20.02.2015 19:06, Radim Krčmář wrote: > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > > - if (!!n->vhost_started == > > - (virtio_net_started(n, status) && !nc->peer->link_down)) { > > + if ((virtio_net_started(n, status) && !nc->peer->link_down) == > > + !!n->vhost_started) { > > btw, can this be rewritten as > (bool)n->vhost_started > instead of > !!n->vhos_started Yes. (It's the same as long as we use bool from stdbool.h.) > Not questioning the patch itself, just wondering, as these > double-negatives look ugly... Casting to bool looks better to me as well, yet the QEMU design guide, `grep $x | wc -l`, greatly prefers bangs. I think that it is best to define it as bool in struct VirtIONet, but I prefer not to change decision that I don't understand ... Thanks for accepting the patches.
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 45da34ad6129..93818675588e 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -120,8 +120,8 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status) return; } - if (!!n->vhost_started == - (virtio_net_started(n, status) && !nc->peer->link_down)) { + if ((virtio_net_started(n, status) && !nc->peer->link_down) == + !!n->vhost_started) { return; } if (!n->vhost_started) { diff --git a/kvm-all.c b/kvm-all.c index 05a79c20e0bb..07ef62cb3227 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -366,7 +366,7 @@ static void kvm_log_stop(MemoryListener *listener, } } -static int kvm_set_migration_log(int enable) +static int kvm_set_migration_log(bool enable) { KVMState *s = kvm_state; KVMSlot *mem; diff --git a/qemu-img.c b/qemu-img.c index e148af8a3e64..21fff2ad53d5 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -977,7 +977,8 @@ static int is_allocated_sectors_min(const uint8_t *buf, int n, int *pnum, static int compare_sectors(const uint8_t *buf1, const uint8_t *buf2, int n, int *pnum) { - int res, i; + bool res; + int i; if (n <= 0) { *pnum = 0;
man gcc: Warn about logical not used on the left hand side operand of a comparison. This option does not warn if the RHS operand is of a boolean type. By preferring bool over int where sensible, but without modifying any depending code, make GCC happy in cases like this, qemu-img.c: In function ‘compare_sectors’: qemu-img.c:992:39: error: logical not is only applied to the left hand side of comparison [-Werror=logical-not-parentheses] if (!!memcmp(buf1, buf2, 512) != res) { hw/ide/core.c:1836 doesn't throw an error, assert(!!s->error == !!(s->status & ERR_STAT)); even thought the second operand is int (and first hunk of this patch has a very similar case), maybe GCC developers still have a little faith in C programmers. Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> --- v2: swapped lines in hunk 1 to to avoid parentheses [Paolo] hw/net/virtio-net.c | 4 ++-- kvm-all.c | 2 +- qemu-img.c | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-)