qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH] nbd: generalize usage of nbd_read


From: Eric Blake
Subject: Re: [Qemu-block] [PATCH] nbd: generalize usage of nbd_read
Date: Fri, 1 Feb 2019 12:27:09 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0

On 1/28/19 10:58 AM, Vladimir Sementsov-Ogievskiy wrote:
> We generally do very similar things around nbd_read: error_prepend,
> specifying, what we have tried to read and be_to_cpu conversion of
> integers.
> 
> So, it seems reasonable to move common things to helper functions,
> which:
> 1. simplify code a bit
> 2. generalize nbd_read error descriptions, all starting with
>    "Failed to read"
> 3. make it more difficult to forget to convert things from BE
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>  include/block/nbd.h | 31 ++++++++++++++--
>  block/nbd-client.c  |  5 ++-
>  nbd/client.c        | 88 +++++++++++++++------------------------------
>  nbd/common.c        |  2 +-
>  nbd/server.c        | 27 +++++---------
>  5 files changed, 70 insertions(+), 83 deletions(-)


> +
> +#define DEF_NBD_READ(bits) \
> +static inline int nbd_read ## bits(QIOChannel *ioc, uint ## bits ## _t * 
> val, \

checkpatch didn't flag it, but I think our style is '*val', not '* val'.

> +                                   const char *desc, Error **errp)           
>  \
> +{                                                                            
>  \
> +    if (nbd_read(ioc, val, sizeof(*val), desc, errp) < 0) {                  
>  \
> +        return -1;                                                           
>  \
> +    }                                                                        
>  \
> +    *val = be ## bits ## _to_cpu(*val);                                      
>  \
> +    return 0;                                                                
>  \
>  }
>  
> +DEF_NBD_READ(16) /* Defines nbd_read16(). */
> +DEF_NBD_READ(32) /* Defines nbd_read32(). */
> +DEF_NBD_READ(64) /* Defines nbd_read64(). */
> +
> +#undef DEF_NBD_READ

I think the name DEF_NBD_READ_N is slightly nicer; that's minor enough
that I don't mind tweaking it.

Reviewed-by: Eric Blake <address@hidden>

Thanks; queued for my next NBD pull request.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]