Hello Stephen,

On 03/25/2014 08:37 PM, Stephen Warren wrote:
On 03/19/2014 11:58 AM, Przemyslaw Marczak wrote:
Those commands basis on implementation of random UUID generator version 4
which is described in RFC4122. The same algorithm is used for generation
both ids but string representation is different as below.

char:  0        9    14   19   24         36
        xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
UUID:     be     be   be   be       be
GUID:     le     le   le   be       be

Commands usage:
- uuid <varname(optional)>
- guid <varname(optional)>

Square brackets are usually used to indicate optional parameters:

- uuid [<varname>]
- guid [<varname>]


Ok, I change this.

diff --git a/include/common.h b/include/common.h

  #if defined(CONFIG_RANDOM_MACADDR) || \
        defined(CONFIG_BOOTP_RANDOM_DELAY) || \
        defined(CONFIG_CMD_LINK_LOCAL) || \
-       defined(CONFIG_RANDOM_UUID)
+       defined(CONFIG_RANDOM_UUID) || \
+       defined(CONFIG_CMD_UUID)

Why not require that if you want to use CONFIG_CMD_UUID, you must define
CONFIG_RANDOM_UUID too? You can even make that automatic in
include/config_fallbacks.h which already does similar things:

#if defined(CONFIG_CMD_FAT) && !defined(CONFIG_FS_FAT)
#define CONFIG_FS_FAT
#endif

That way, you won't need to touch lib/Makefile in this patch either, or
modify the ifdef that wraps gen_rand_uuid().


I change this part of code in one of my other patch set which can be found here: http://patchwork.ozlabs.org/patch/332499/
After apply those changes then I add some automation here.

diff --git a/lib/uuid.c b/lib/uuid.c


+#ifdef CONFIG_CMD_UUID
+int do_uuid(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+       char uuid[UUID_STR_LEN + 1];
+       uuid_str_t str_format;
+
+       if (!strcmp(argv[0], "uuid"))
+               str_format = UUID_STR_FORMAT_STD;
+       else
+               str_format = UUID_STR_FORMAT_GUID;
+
+       if (argc == 1) {
+               gen_rand_uuid_str(uuid, str_format);
+               printf("%s\n", uuid);
+       } else if (argc == 2) {
+               gen_rand_uuid_str(uuid, str_format);
+               setenv(argv[1], uuid);
+       } else {
+               return CMD_RET_USAGE;
+       }

This duplicates some code; the call to gen_rand_uuid(). I think it would
be better as:

if (argc < 2)
        return CMD_RET_USAGE;
gen_rand_uuid_str(uuid, str_format);
if (argc == 1)
        printf("%s\n", uuid);
else
        setenv(argv[1], uuid);


Yes, this is better, but the first condition should be as:
if ((argc != 1) || (argc != 2))

+U_BOOT_CMD(uuid, CONFIG_SYS_MAXARGS, 1, do_uuid,
+       "UUID - generate Universally Unique Identifier version 4",

Would it be batter to say "a random ..." rather than "... version 4"?
I'm not sure if the details of the version matter so long as its a valid
UUID, and certainly the fact the generated UUID is random is likely more
interesting.

+       "<varname(optional)>\n"

        "[<varname>]\n"


Ok, I also apply those two commands above.

Thanks
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marc...@samsung.com
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to