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

Re: [PATCH v2 3/3] nbd: Use uint64_t instead of char[8] for cookie



On Thu, Mar 09, 2023 at 03:06:23PM -0600, Eric Blake wrote:
> Type-punning *(uint64_t*)(char[8]) is unsafe if the pointer is not
> aligned per the requirements of the hardware (x86_64 has 1-byte
> alignment, but other hardware exists that will give a SIGBUS).  In
> practice, it didn't matter - 'struct nbd_request' was already 64-bit
> aligned for 'from' (even before the recent change in commit 739f7121
> to pack it), and 'struct nbd_reply' being packed means the compiler
> emits code to deal with 1-byte alignment despite hardware.  But since
> the cookie is already opaque on the server side, we might as well
> treat it as an 8-byte integer instead of a character array, with no
> visible semantic changes to the client.

Sheesh - not sure why my git setup failed me.  If it helps:

Signed-off-by: Eric Blake <eblake@redhat.com>

> --- a/tests/run/nbd-tester-client.c
> +++ b/tests/run/nbd-tester-client.c

> @@ -720,8 +720,8 @@ int read_packet_check_header(int sock, size_t datasize, long long int curcookie)
>  			 "Received package with incorrect reply_magic. Index of sent packages is %lld (0x%llX), received cookie is %lld (0x%llX). Received magic 0x%lX, expected 0x%lX",
>  			 (long long int)curcookie,
>  			 (long long unsigned int)curcookie,
> -			 (long long int)*((u64 *) rep.cookie),
> -			 (long long unsigned int)*((u64 *) rep.cookie),
> +			 (long long int)rep.cookie,
> +			 (long long unsigned int)rep.cookie,

Is it worth using PRIX64 to cut out the need for casts in these
printf's?  That's a separate cleanup, if desired.

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


Reply to: