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.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
