[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <200910161103.40689.laurent.pinchart@ideasonboard.com>
Date: Fri, 16 Oct 2009 11:03:40 +0200
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Huang Ying <ying.huang@...el.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Joe Perches <joe@...ches.com>,
Steven Whitehouse <swhiteho@...hat.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC 1/2] Unified UUID/GUID definition
Hi Huang,
thanks for having incorporated my comments into this second round of patches.
A one more comment inline.
On Wednesday 14 October 2009 08:30:05 Huang Ying wrote:
> There are many different UUID/GUID definitions in kernel, such as that
> in EFI, many file systems, some drivers, etc. Every kernel components
> need UUID/GUID has its own definition. This patch provides a unified
> definition for UUID/GUID.
>
> UUID is defined via typedef. This makes that UUID appears more like a
> preliminary type, and makes the data type explicit (comparing with
> implicit "u8 uuid[16]").
>
> The binary representation of UUID/GUID can be little-endian (used by
> EFI, etc) or big-endian (defined by RFC4122), so both is defined.
>
> Signed-off-by: Huang Ying <ying.huang@...el.com>
> ---
> include/linux/uuid.h | 59
> +++++++++++++++++++++++++++++++++++++++++++++++++++ lib/Makefile |
> 2 -
> lib/uuid.c | 36 +++++++++++++++++++++++++++++++
> 3 files changed, 96 insertions(+), 1 deletion(-)
> create mode 100644 include/linux/uuid.h
> create mode 100644 lib/uuid.c
>
> --- /dev/null
> +++ b/include/linux/uuid.h
> @@ -0,0 +1,59 @@
> +/*
> + * Unified UUID/GUID definition
> + *
> + * Copyright (C) 2009, Intel Corp.
> + * Huang Ying <ying.huang@...el.com>
> + *
> + * This file is released under the GPLv2.
> + */
> +
> +#ifndef _LINUX_UUID_H_
> +#define _LINUX_UUID_H_
> +
> +#include <linux/types.h>
> +#include <linux/string.h>
> +
> +typedef struct {
> + __u8 b[16];
> +} uuid_le;
> +
> +typedef struct {
> + __u8 b[16];
> +} uuid_be;
> +
> +#define UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
> +((uuid_le) \
> +{{ (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff,
> \ + (b) & 0xff, ((b) >> 8) & 0xff, \
> + (c) & 0xff, ((c) >> 8) & 0xff, \
> + (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
> +
> +#define UUID_BE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
> +((uuid_be) \
> +{{ ((a) >> 24) & 0xff, ((a) >> 16) & 0xff, ((a) >> 8) & 0xff, (a) & 0xff,
> \ + ((b) >> 8) & 0xff, (b) & 0xff, \
> + ((c) >> 8) & 0xff, (c) & 0xff, \
> + (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
> +
> +#define NULL_UUID_LE \
> + UUID_LE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00, \
> + 0x00, 0x00, 0x00, 0x00)
> +
> +#define NULL_UUID_BE \
> + UUID_BE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00, \
> + 0x00, 0x00, 0x00, 0x00)
> +
> +static inline int uuid_le_cmp(const uuid_le u1, const uuid_le u2)
> +{
> + return memcmp(&u1, &u2, sizeof(uuid_le));
> +}
> +
> +static inline int uuid_be_cmp(const uuid_be u1, const uuid_be u2)
> +{
> + return memcmp(&u1, &u2, sizeof(uuid_be));
> +}
Shouldn't you pass pointers to those functions instead ?
It would also be nice to have a single uuid_cmp function. Would it be possible
to use typeof(), __builtin_types_compatible_p() and BUILD_BUG_ON_ZERO magic to
make sure both arguments are of the same type and one of uuid_be and uuid_le ?
It might no work with the Intel compiler though.
> +extern void uuid_le_gen(uuid_le *u);
> +extern void uuid_be_gen(uuid_be *u);
> +
> +#endif
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -21,7 +21,7 @@ lib-y += kobject.o kref.o klist.o
>
> obj-y += bcd.o div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o
> \ bust_spinlocks.o hexdump.o kasprintf.o bitmap.o scatterlist.o \ -
> string_helpers.o gcd.o
> + string_helpers.o gcd.o uuid.o
>
> ifeq ($(CONFIG_DEBUG_KOBJECT),y)
> CFLAGS_kobject.o += -DDEBUG
> --- /dev/null
> +++ b/lib/uuid.c
> @@ -0,0 +1,36 @@
> +/*
> + * Unified UUID/GUID definition
> + *
> + * Copyright (C) 2009, Intel Corp.
> + * Huang Ying <ying.huang@...el.com>
> + *
> + * This file is released under the GPLv2.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/uuid.h>
> +#include <linux/random.h>
> +
> +static void __uuid_gen_common(__u8 b[16])
> +{
> + get_random_bytes(b, 16);
> + /* reversion 0b10 */
> + b[8] = (b[8] & 0x3F) | 0x80;
> +}
> +
> +void uuid_le_gen(uuid_le *lu)
> +{
> + __uuid_gen_common(lu->b);
> + /* version 4 : random generation */
> + lu->b[7] = (lu->b[7] & 0x0F) | 0x40;
> +}
> +EXPORT_SYMBOL_GPL(uuid_le_gen);
> +
> +void uuid_be_gen(uuid_be *bu)
> +{
> + __uuid_gen_common(bu->b);
> + /* version 4 : random generation */
> + bu->b[6] = (bu->b[6] & 0x0F) | 0x40;
> +}
> +EXPORT_SYMBOL_GPL(uuid_be_gen);
>
--
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists