Fix erroneous double negation in conditional

Commit Message

Raphael Norwitz May 7, 2020, 8:06 p.m. UTC

In vhost_migration_log() there is the following check:
    if(!!enable == dev->log_enabled) {
        return 0;
    }

The double negative “!!” is unnecessary and bad coding style. This
change removes it.

Signed-off-by: Raphael Norwitz <[e-mail]>
---
 hw/virtio/vhost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

     if (!dev->started) {
--
1.8.3.1

Comments

Eric Blake May 7, 2020, 8:28 p.m. UTC | #1

On 5/7/20 3:06 PM, Raphael Norwitz wrote:
> In vhost_migration_log() there is the following check:
>      if(!!enable == dev->log_enabled) {
>          return 0;
>      }
> 
> The double negative “!!” is unnecessary and bad coding style. This
> change removes it.

!!int or !!ptr is not bad coding style - it is the shortest way to 
compare a non-bool against 0, and canonicalize the result back into bool 
(that is, convert all non-zero values into '1').  But !!bool is a waste 
of typing, since bool is already in the proper form.  Your patch as-is 
is incorrect; since the function declares 'int enable', this is using 
the !!int form which is not bad coding style.

On the other hand, looking at this function closer, we see that 
vhost_migration_log() is static, so all uses lie within this file.  And 
the callers are:

static void vhost_log_global_start(MemoryListener *listener)
     r = vhost_migration_log(listener, true);
static void vhost_log_global_stop(MemoryListener *listener)
     r = vhost_migration_log(listener, false);

and looking at struct vhost_dev, its log_enabled member is bool.

So the _real_ problem with this file is that it uses 'int enable' rather 
than 'bool enable'.  And once you fix the parameter type, then you are 
indeed correct that you would have a !!bool scenario worth cleaning up.

Looking forward to v2 along those lines.

Patch

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index aff98a0..83be0c8 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -814,7 +814,7 @@  static int vhost_migration_log(MemoryListener
*listener, int enable)
     struct vhost_dev *dev = container_of(listener, struct vhost_dev,
                                          memory_listener);
     int r;
-    if (!!enable == dev->log_enabled) {
+    if (enable == dev->log_enabled) {
         return 0;
     }