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

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2156 bytes --]

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 <vsementsov@virtuozzo.com>
> ---
>  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 <eblake@redhat.com>

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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]