Re: [Qemu-devel] [PATCH v2 1/3] io: Fix double shift usages on QIOChannel features

["Daniel P. Berrange" <berrange@redhat.com>]

On Wed, Sep 28, 2016 at 05:36:05AM -0700, Felipe Franciosi wrote:
> When QIOChannels were introduced in 666a3af9, the feature bits were
> already defined shifted. However, when using them, the code was shifting
> them again. The incorrect use was consistent until 74b6ce43, where
> QIO_CHANNEL_FEATURE_LISTEN was defined shifted but tested unshifted.
> 
> This patch changes the definition to be unshifted and fixes the
> incorrect usage introduced on 74b6ce43.
> 
> Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
> ---
>  include/io/channel.h |    6 +++---
>  io/channel-socket.c  |    2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/io/channel.h b/include/io/channel.h
> index 752e89f..5368604 100644
> --- a/include/io/channel.h
> +++ b/include/io/channel.h
> @@ -40,9 +40,9 @@ typedef struct QIOChannelClass QIOChannelClass;
>  typedef enum QIOChannelFeature QIOChannelFeature;
>  
>  enum QIOChannelFeature {
> -    QIO_CHANNEL_FEATURE_FD_PASS  = (1 << 0),
> -    QIO_CHANNEL_FEATURE_SHUTDOWN = (1 << 1),
> -    QIO_CHANNEL_FEATURE_LISTEN   = (1 << 2),
> +    QIO_CHANNEL_FEATURE_FD_PASS,
> +    QIO_CHANNEL_FEATURE_SHUTDOWN,
> +    QIO_CHANNEL_FEATURE_LISTEN,
>  };
>  
>  
> diff --git a/io/channel-socket.c b/io/channel-socket.c
> index 196a4f1..6710b2e 100644
> --- a/io/channel-socket.c
> +++ b/io/channel-socket.c
> @@ -403,7 +403,7 @@ static void qio_channel_socket_finalize(Object *obj)
>      QIOChannelSocket *ioc = QIO_CHANNEL_SOCKET(obj);
>  
>      if (ioc->fd != -1) {
> -        if (QIO_CHANNEL(ioc)->features & QIO_CHANNEL_FEATURE_LISTEN) {
> +        if (QIO_CHANNEL(ioc)->features & (1 << QIO_CHANNEL_FEATURE_LISTEN)) {
>              Error *err = NULL;
>  
>              socket_listen_cleanup(ioc->fd, &err);

The change looks good, but this patch is missing additions to the
various tests/test-io-channel-*.c files, to validate that we correctly
report the SHUTDOWN/LISTEN features being set.

Any test addition should fail on current git master, and succeed with
this fix applied.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|