Re: [PATCH v1 1/1] cmd: setexpr: add dec operation for converting variable to decimal

[Wolfgang Denk <wd@denx.de>]

Dear Roland,

In message <d057362e-b317-0361-92b0-b8b88ee38e3c@weidmueller.com> you wrote:
>
> > ...
> >> +     /* hexadecimal to decimal conversion: "setexpr name dec value" */
> >> +     if (argc == 4 && (strcmp(argv[2], "dec") == 0)) {
> >> +             w = cmd_get_data_size(argv[3], 4);
> >> +             a = get_arg(argv[3], w);
> >> +             return env_set_ulong(argv[1], a);
> >> +     }
> > 
> > Should there not be a test for 4 arguments and the third _not_ being
> > "dec" ?  Like "setexpr foo hex 42" ?
>
> Yes it's possible to add further conversion operations. But I didn't saw a
> need for hex in the first place, as hex is currently the default within

You misunderstand.  With your code, the incorect command "setexpr foo hex 42"
would not raise any error message.

Assume such a call:

	setexpr foo kjkjkjlkj 42

This should raise an error, right?


Instead of

	if (argc == 4 && (strcmp(argv[2], "dec") == 0)) {
		...
	}

you would need something like:

	if (argc == 4) {
		if strcmp((argv[2], "dec") != 0) {
			print error message
			bail our
		}
		...
	}

But see my other suggestion anyway - why add just decimal format
when using sprintf() with a format string allows for all kinds of
fancy uses?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
"Love is an ideal thing, marriage a real thing; a  confusion  of  the
real with the ideal never goes unpunished."                  - Goethe