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

[Roland Gaudig (OSS)]

Hello Wolfgang

On 29.06.21 10:40, Wolfgang Denk wrote:
> 
> Dear Roland,
> 
> In message <a463f32f-8ef0-6973-f1c3-a881ee6e5d26 at weidmueller.com> you wrote:
>>
>>> 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 possible to allow more than one format parameter or more
>> types. But it would make checking much more difficult.
> 
> Maybe we need _less_ checking, not more - and maybe the needed
> checking is already done in the *printf() code?

The problem printf does not do much checking. For example in case the
format string does not match the number of arguments or argument types
in best case it just delivers a wrong result, but the program also can
just crash. That is why I added the checks.

In contrast Bash and Busybox are reporting error messages in the above
cases.

>> I think just passing the format string directly to sprintf should be
>> avoided because it is unsafe. For example
>>
>> => setexpr foo fmt %s 0xffffffff
>>
>> would surely lead to access on memory location outside the variable
>> where 0xffffffff is stored.
> 
> Only if you make the wrong assumptions.  I would expect this to
> result in
> 
>         foo=0xffffffff
> 
> in the same way as the bash builting gives
> 
>         $ printf '%s\n' 0xffffffff
>         0xffffffff

Yes, but that requires further checks and interpretation. To maintain
the possibility to use pointers as arguments, the get_arg() function is
necessary, but in the above example it would return a ulong which needs
to be converted to a string before passing to printf, to get the above
result.

>>> => setexpr foo fmt "%0x08x-%s-%d-%s" $a $b $c $d
>>
>> I think the only way to support such expressions in a save way would
>> be implementing an own format string parser for setexpr with
> 
> Maybe it makes sense to have a look at the bash code?

I looked at Bash code but it is quite confusing as they implement at
least three format string parsers. I think the one relevant for us is
the function printf_builtin() inside builtins/printf.dev. It has a
length of about 450 lines.

I also had a look at the busybox shell. They implemented their own
format string parser too, which is also about 450 lines long.

I don't see a leaner way for implementing a Bash like printf
functionality with multiple arguments and all kinds of supported format
types. When adding that format string capabilities in my opinion it
should be a configuration option to keep code size low on systems not
needing that functionality. Also I would tend to make the specific
format string features configurable. For example in my application only
decimal conversion is needed, also enabling floating point support
would just increase code without bringing any benefits.

Best regards,
Roland Gaudig