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

[Sean Anderson <seanga2@gmail.com>]

On 6/29/21 4:37 AM, Wolfgang Denk wrote:
> Dear Roland,
> 
> In message <20210628151750.572837-1-roland.gaudig-oss@weidmueller.com> you wrote:
>>
>>
>> U-Boot uses almost everywhere hexadecimal numbers. But some bootargs
>> passed to Linux are expecting decimal numbers. As long as the values
>> are in the range 0 to 9 it is sufficient to just strip 0x from the
>> number. But for greater values a method for converting numbers to
>> decimal is needed.
>>
>> This patch adds C like format string capabilities to the setexpr
>> command. Here are some examples:
> 
> Thanks!
> 
>> In contrast to the original C format strings the number of parameters
>> is limited to one. As the get_arg() function returns unsigned long
>> type, the format string commands are limited to those which are
>> operating on unsigned long type.
> 
> These are two pretty unfortunate restrictions.  I guess it should
> not be too hard to avoid both of these.  Can you please give it a
> try?
> 
> I think it is reasonable to assume (and specify) that, when the
> "fmt" option is used, _all_ following arguments will be passed
> (unchanged) to the sprintf() function.
> 
> This was actually one of my intentions when making this suggestion -
> to be able to construct any kind of data from pieces; say, for
> example:
> 
> => setexpr foo fmt "%0x08x-%s-%d-%s" $a $b $c $d
> 
> Best regards,
> 
> Wolfgang Denk
> 

The issue with this is twofold. First, there is no portable way to
construct a va_list from C code. So the likely way to do this would be
to set an arbitrary limit, and then just pass the arguments in. E.g.
something like

	snprintf(buf, sizeof(buf), argv[3], argc >= 4 ? argv[4] : NULL, /* etc */);

but of course there is no way to check that the format string matches
the correct number of arguments. This is a pretty big footgun.

The other problem is that things like `%d` expect a number and not a
string. So you would have to reimplement snprintf anyway so that it
expects all of its arguments to be strings, and calls strtoul as
appropriate.  And considering that the *printf functions take 5k
already, this reimplementation may add a significant amount of code.
For this reason, I'd much prefer to just have `hex` and `dec` functions
which do the appropriate conversions.

--Sean