On 6/29/21 11:13 AM, Wolfgang Denk wrote:
Dear Sean,

In message <19b6eeea-2aad-972b-aeeb-8959aab17...@gmail.com> you wrote:

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

We already have an argument list: it's what's being passed to the
"setexpr" command, minus the initial arguments.

        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.


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.

You have this problem always when you have user provided format
strings and arguments.  We don't have to re-invent the wheel here.
I repeat myself: maybe we should have a look at bash's
implementation of the printf builtin command?  there I get for
example this:

        $ printf "%d %d %d\n" 3
        3 0 0
        $ printf "%d %d %d\n" foo bar
        -bash: printf: foo: invalid number
        -bash: printf: bar: invalid number
        0 0 0
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.

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.

--Sean

Reply via email to