On 08/17/2016 02:28 AM, Fam Zheng wrote: > A number of different places across the code base use CONFIG_UUID. Some > of them are soft dependency, some are not built if libuuid is not > available, some come with dummy fallback, some throws runtime error. > > It is hard to maintain, and hard to reason for users. > > Since UUID is a simple standard with only a small number of operations, > it is cleaner to have a central support in libqemuutil. This patch adds > qemu_uuid_* the functions so that all uuid users in the code base can
s/the functions so/functions/ > rely on. Except for qemu_uuid_generate which is new code, all other > functions are just copy from existing fallbacks from other files. > > Note that qemu_uuid_parse is moved without updating the function > signature to use QemuUUID, to keep this patch simple. > > Signed-off-by: Fam Zheng <f...@redhat.com> > --- > +++ b/include/qemu/uuid.h > @@ -0,0 +1,59 @@ > +/* > + * QEMU UUID functions > + * > + * Copyright 2016 Red Hat, Inc., Why the trailing comma? > +++ b/util/uuid.c > @@ -0,0 +1,92 @@ > +/* > + * QEMU UUID functions > + * > + * Copyright 2016 Red Hat, Inc., copy-and-paste? :) > + > +#include "qemu/osdep.h" > +#include "qemu-common.h" > +#include "qemu/uuid.h" > +#include "qemu/bswap.h" > +#include <glib.h> Do you still need the <glib.h> header here? And if so, shouldn't it be right after osdep.h? > + > +void qemu_uuid_generate(QemuUUID *uuid) > +{ > + int i; > + uint32_t tmp[4]; > + > + QEMU_BUILD_BUG_ON(sizeof(QemuUUID) != 16); > + > + for (i = 0; i < 4; ++i) { > + tmp[i] = g_random_int(); > + } > + memcpy(uuid, tmp, sizeof(tmp)); > + /* 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; > +} Looks okay. > + > +int qemu_uuid_is_null(const QemuUUID *uu) > +{ > + QemuUUID null_uuid = { 0 }; Could make this static, so that it doesn't have to be zeroed on every entry to this function (but in a separate patch, since this one just moved it, right?) > + return memcmp(uu, &null_uuid, sizeof(QemuUUID)) == 0; > +} > + Once the nits are cleaned up, you can add: Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature