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

Re: [Libguestfs] [PATCH nbdkit 4/4] common/protocol: Install <nbd-protocol.h> as a public header.



On 9/24/19 4:07 PM, Richard W.M. Jones wrote:
> Some further small changes are made to allow this header to be used
> from arbitrary ISO C compilers.
> 
> It can now be used from other projects.
> ---

>  
> +#if defined(__GNUC__) || defined(__clang__)
> +#define NBD_ATTRIBUTE_PACKED __attribute__((__packed__))
> +#else
> +#define NBD_ATTRIBUTE_PACKED
> +#endif
> +

Actually, I just realized that this fallback is wrong - it induces
padding bytes, and thus creates a struct incompatible with the wire
format.  We're better off making the #else use
#error Please port to your compiler's notion of a packed struct

so that the file serves as a reference, but does not get mistakenly used
with incorrect padding.

>  #define NBD_MAX_STRING 4096 /* Maximum length of a string field */
>  
>  /* Old-style handshake. */
> @@ -50,7 +56,7 @@ struct nbd_old_handshake {
>    uint16_t gflags;            /* global flags */
>    uint16_t eflags;            /* per-export flags */
>    char zeroes[124];           /* must be sent as zero bytes */
> -} __attribute__((packed));
> +} NBD_ATTRIBUTE_PACKED;

This one actually gets wire alignment without packing.

>  
>  #define NBD_OLD_VERSION UINT64_C(0x420281861253)
>  
> @@ -59,7 +65,7 @@ struct nbd_new_handshake {
>    char nbdmagic[8];           /* "NBDMAGIC" */
>    uint64_t version;           /* NBD_NEW_VERSION */
>    uint16_t gflags;            /* global flags */
> -} __attribute__((packed));
> +} NBD_ATTRIBUTE_PACKED;

This one would get tail padding, messing up anyone using sizeof(struct
nbd_new_handshake).

>  
...

> @@ -140,19 +146,19 @@ struct nbd_fixed_new_option_reply_info_export {
>    uint16_t info;                /* NBD_INFO_EXPORT */
>    uint64_t exportsize;          /* size of export */
>    uint16_t eflags;              /* per-export flags */
> -} __attribute__((packed));
> +} NBD_ATTRIBUTE_PACKED;
>  

and here's one that gets member offsets wrong, if packed is not used.


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

Attachment: signature.asc
Description: OpenPGP digital signature


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