lkml.org 
[lkml]   [2012]   [Oct]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 2/9] uuid: use random32_get_bytes()
From
Date
On Tue, 2012-10-30 at 22:38 -0400, Theodore Ts'o wrote:
> On Wed, Oct 31, 2012 at 09:35:37AM +0800, Huang Ying wrote:
> >
> > The intention of lib/uuid.c is to unify various UUID related code, and
> > put them in same place. In addition to UUID generation, it provide some
> > other utility and may provide/collect more in the future. So do you
> > think it is a good idea to put generate_rand_uuid/guid into lib/uuid.c
> > and maybe change the name/prototype to make it consistent with other
> > uuid definitions?
>
> I had trouble understanding why lib/uuid.c existed, since the only
> thing I saw was the uuid generation function. After some more
> looking, I see you also created inline functions which wrapped
> memcmp().
>
> The problem I have with your abstractions is that it just makes life
> more complicated for the callers. All of the current places which use
> generate_random_uuid() merely want to fill in a unsigned char array.
> This includes btrfs, by the way, which is already using
> generate_random_uuid in some places, and I'm not sure why they are
> using uuid_le_gen(), since there doesn't seem to be any need for a
> little-endian uuid/guid here (it's just used as unique bag of bits
> which is 16 bytes long), and using uuid_le_gen() means extra memory
> has to be allocated on the stack, and then an extra memory copy is
> required. Contrast (in fs/btrfs/root-tree.c):
>
> uuid_le uuid;
> ...
> uuid_le_gen(&uuid);
> memcpy(item->uuid, uuid.b, BTRFS_UUID_SIZE);
>
> versus, simply doing (fs/btrfs/volumes.c):
>
> generate_random_uuid(fs_devices->fsid);
>
> see which one is easier? And after the uuid is generated, none of the
> current callers ever do any manipulation of the uuid, so there's no
> real point to play fancy typedef games; it just adds more work for no
> real gain.

If we use uuid_le when we define the data structure, life will be eaiser

struct btrfs_root_item {
...
uuid_le uuid;
...
};

Then it is quite easy to use it.

uuid_le_gen(&item->uuid);

That is the intended usage model.

UUID_LE() macro definition has some user. It makes it easier to
construct UUID/GUID defined in some specs.

> > > Using UUID vs. GUID I think makes things much clearer, since the EFI
> > > specification talks about GUID's, not UUID's, and that way we don't
> > > have to worry about people getting confused about whether they should
> > > be using the little-endian versus big-endian variant. (And I'd love
> > > to ask to whoever wrote the EFI specification what on *Earth* were
> > > they thinking when they decided to diverge from the rest of the
> > > world....)
> >
> > I think that is a good idea. From Wikipedia, GUID is in native byte
> > order, while UUID is in internet byte order.
>
> Well, technially GUID is "intel/little-endian byte order". If someone
> tried to implement the GPT on a big-endian system, such as PowerPC,
> they would still have to use the little-endian byte order, even though
> it's not the native byte order for that architecture. Otherwise
> devices wouldn't be portable between those systems. (This is why I
> think the GUID was such a bad idea; everyone basically treats them as
> 16 byte octet strings, so this whole idea of "native byte order" just
> to save a few byte swaps at UUID generation time was really, IMHO, a
> very bad idea.)

Yes. Explicit byte order is better.

Best Regards,
Huang Ying




\
 
 \ /
  Last update: 2012-10-31 04:41    [W:0.040 / U:0.304 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site