[U-Boot] [PATCH 1/1] lib: uuid: alignment error in gen_rand_uuid()

[Heinrich Schuchardt]

On 7/15/19 10:42 AM, Eugeniu Rosca wrote:
> Hi Heinrich,
>
> On Sun, Jul 14, 2019 at 11:31:50PM +0200, Heinrich Schuchardt wrote:
>> Packed structures like struct uuid are not aligned. GCC 9.1 therefore
>> throws an error when trying to compile gen_rand_uuid().
>>
>> lib/uuid.c: In function ‘gen_rand_uuid’:
>> lib/uuid.c:244:2: error: converting a packed ‘struct uuid’ pointer
>> (alignment 1) to a ‘unsigned int’ pointer (alignment 4) may result in
>> an unaligned pointer value [-Werror=address-of-packed-member]
>>    244 |  unsigned int *ptr = (unsigned int *)&uuid;
>>        |  ^~~~~~~~
>>
>> Generate the uuid in a properly aligned buffer.
>>
>> The byte order of a random number should not matter. Do not call
>> cpu_to_be32() to change the byte order.
>
> There is a striking resemblance between "struct uuid" [1] in U-Boot and
> "struct afs_uuid" [2] in Linux, with the major difference that the
> latter has never been defined as __packed.
>
> Is there any compelling reason for the U-Boot struct to be packed?

Thanks for your analysis.

struct uuid is only used in lib/uuid.c inside gen_rand_uuid(). Hence
there is no need make this structure packed. We should still remove the
superfluous call to cpu_to_be32().

Best regards

Heinrich


>
> [1] https://gitlab.denx.de/u-boot/u-boot/commit/4e4815feae4d3
>      ("lib: uuid: add functions to generate UUID version 4")
>   -----8<-----
>   struct uuid {
>   	unsigned int time_low;
>   	unsigned short time_mid;
>   	unsigned short time_hi_and_version;
>   	unsigned char clock_seq_hi_and_reserved;
>   	unsigned char clock_seq_low;
>   	unsigned char node[6];
>   } __packed;
>   -----8<-----
>
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f044c8847bb6
>   -----8<-----
>   struct afs_uuid {
>   	__be32		time_low;			/* low part of timestamp */
>   	__be16		time_mid;			/* mid part of timestamp */
>   	__be16		time_hi_and_version;		/* high part of timestamp and version  */
>   	__u8		clock_seq_hi_and_reserved;	/* clock seq hi and variant */
>   	__u8		clock_seq_low;			/* clock seq low */
>   	__u8		node[6];			/* spatially unique node ID (MAC addr) */
>   };
>   -----8<-----
>