[U-Boot] [PATCH v3 3/3] cmd:gpt: randomly generate each partition uuid if undefined

[Wolfgang Denk]

Dear Przemyslaw Marczak,

In message <e7d9246677ecb9a8c774e9e5f8d31ae3fd53d487.1394807506.git.p.marczak at samsung.com> you wrote:
> Changes:
> - randomly generate partition uuid if any is undefined
> - print info about set/unset/generated uuid
> - update doc/README.gpt
...
> +	int ret = -1;
>  	char *e, *s;
> +	char uuid_str[37];

Should we not rather use a #defined macro here instead of the magic
number 37 ?

> -			printf("Environmental '%s' not set\n", str);
> -			return -1; /* env not set */
> +			printf("%s ", str);
> +			gen_rand_uuid_str(uuid_str);
> +			setenv(s, uuid_str);
> +
> +			e = getenv(s);
> +			if (e) {
> +				puts("set to random.\n");

Can we keep the "var not set" part, so the user understands why U-Boot
assigns a random ID here?

> +		} else {
> +			printf("%s get from environment.\n", str);

Make this debug() ?

> +		puts("Writing GPT:\n");
> +
> +		ret = gpt_default(blk_dev_desc, argv[4]);
> +		if (!ret) {
> +			puts("success!\n");
> +			return CMD_RET_SUCCESS;
> +		} else {
> +			puts("error!\n");
>  			return CMD_RET_FAILURE;

This code is too verbose - I suggest to turn all these puts() into
debug().

>  	} else {
>  		return CMD_RET_USAGE;
>  	}

Please invert the logic so you can bail out early and reduce the
indentation level.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Work 8 hours, sleep 8 hours; but not the same 8 hours.