On Fri, 08/12 07:24, Richard Henderson wrote:
> On 08/12/2016 05:52 AM, Fam Zheng wrote:
> >+/* Version 4 UUID (pseudo random numbers), RFC4122 4.4. */
> >+
> >+typedef struct {
> >+ union {
> >+ unsigned char data[16];
> >+ struct {
> >+ /* Generated in BE endian, can be swapped with qemu_uuid_bswap.
> >*/
>
> Nit: BE endian is redundant. big-endian or just BE, please.
>
> >+ uint32_t time_low;
> >+ uint16_t time_mid;
> >+ uint16_t time_high_and_version;
> >+ uint8_t clock_seq_and_reserved;
> >+ uint8_t clock_seq_low;
> >+ uint8_t node[6];
> >+ } fields;
> >+ };
> >+} QemuUUID;
>
> Wait, didn't you just tell me that you *couldn't* add this union
> because of the packed usage within some file?
I was just saying even with this union added, packed usage can still result in
unaligned QemuUUID allocation, in theory. That's why I said I could assert too.
(In practise the only qemu_uuid_bswap is putting QemuUUID in a packed struct,
but that is happily aligned.)
>
> But of course if you're going to do this, you should actually use these
> fields,
>
> >+ /* Set the two most significant bits (bits 6 and 7) of the
> >+ clock_seq_hi_and_reserved to zero and one, respectively. */
> >+ uuid->data[8] = (uuid->data[8] & 0x3f) | 0x80;
> >+ /* Set the four most significant bits (bits 12 through 15) of the
> >+ time_hi_and_version field to the 4-bit version number.
> >+ */
> >+ uuid->data[6] = (uuid->data[6] & 0xf) | 0x40;
>
> Here,
Using these fields here requires an extra cpu_to_be16s for no compelling
reason, so I refrained to.
Fam
>
> >+void qemu_uuid_bswap(QemuUUID *uuid)
> >+{
> >+ assert(QEMU_IS_ALIGNED((uint64_t)uuid, sizeof(uint32_t)));
> >+ bswap32s(&uuid->fields.time_low);
> >+ bswap16s(&uuid->fields.time_mid);
> >+ bswap16s(&uuid->fields.time_high_and_version);
>
> And then you don't need this assert.
>
>
> r~
>
