Re: [PATCH 0/3] cmd: setexpr: add fmt format string operation

[Wolfgang Denk <wd@denx.de>]

Dear Sean,

In message <b85f202f-8527-b149-58d5-7ee09081d645@gmail.com> you wrote:
>
> >> 	snprintf(buf, sizeof(buf), argv[3], argc >= 4 ? argv[4] : NULL, /* etc */);
> > 
> > Why this test on argc?  If it's less than 4, argv[4] should be NULL
> > anyway.
>
> 	snprintf(buf, sizeof(buf), argv[3], argc >= 4 ? argv[4] : NULL,
> 		 argc >= 5 ? argv[5] : NULL, argc >= 6 ? argv[6] : NULL, /* etc */);
>
> and you keep doing this until you get to whatever number of arguments
> you'd like.

I'm sorry, but this is not an acceptable way to implement variadic
functions in C.  Are you aware how va_arg works?

> > Eventually the format checking can be kept out of the generic
> > *printf() code; it could then be optional/configurable with the
> > "fmt" option in the setexpr command.
>
> It's not a "checking" problem. The issue is that "123" cannot
> be passed directly to %d. So you have dig into the guts of snprintf
> anyway.

Did you read my recommendation to have a look for example at the
implementation of the printf bash builting?

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
The moral of the story is: "Don't stop to  tighten  your  shoe  laces
during the Olympics 100m finals".
                             - Kevin Jones in <DEJo68.K1t@bri.hp.com>