[Rd] incorrect output and segfaults from sprintf with %*d (PR#13667)

[Waclaw.Marcin.Kusnierczyk at idi.ntnu.no]

Martin Maechler wrote:
>
>>>     MM> well, it is basically (+ a few bytes ?)
>>>     MM> the same  8192  limit that *is* documented.
>>>
>>> indeed, I was right with that..
>>>
>>>       
>> hmm, i'd guess this limit is valid for all strings included in the
>> output with any format?  not just %s (and, as it appears, undocumentedly
>> %d)?
>>     
>
> yes.
>   

so it's perhaps easiest to change

" There is a limit of 8192 bytes on elements of 'fmt' and also on
     strings included by a '%s' conversion specification."

to sth like

" There is a limit of 8192 bytes on elements of 'fmt' and also on
     strings included in the output by any conversion specification."

it's in fact so easy that even i should be able to do it.

[1 minute later...]  i see you've fixed this one, too.


>> btw. (i do know what that means ;)), after your recent fix:
>>
>>    sprintf('%q%s', 1)
>>    # Error in sprintf("%q%s", 1) :
>>    #  use format %f, %e, %g or %a for numeric objects
>>
>>    sprintf('%s', 1)
>>    # [1] "1"
>>
>> you may want to add '%s' (and '%x', and ...) to the error message.  or
>> perhaps make it say sth like 'invalid format: ...'.  the problem is not
>> that %q is not applicable to numeric, but that it is not a valid format
>> at all.
>>     
>
> yes.  As a matter of fact,  "%q%s" is dealt with as *one* format
> chunk, since "%q" is
> not a valid format.
> The code I have just committed now gives a longer erro message, that
> should be more helpful.
>   

yes, but

    sptinf('%q%s', 1)


still suggests that one uses %{f,e,g,a} for numerics, while %s is pretty
much valid, too.  you see, in c sprintf(buffer, "%s", 1) is destined to
cause a segfault, but in r it works -- so the error message is slightly
misleading, as it suggests %s is *not* valid for numerics.

> Thank you for the suggestion!
>   

yo welcum

>   
>> there's also an issue with the additional arguments supplied after the
>> format:  any superfluous arguments are ignored (this is not documented,
>> as far as i can see),
>>     
>
> I think we should signal an error if there are too many arguments.
>   

agree.  but it might be more complex than it appears:

    sprintf('%3$s', 1, 2, 3)

should *not* complain about too many args, despite just one conversion
spec in the format.  interestingly,

    sprintf('%3$s', , , 3)
    # error: argument is missing, with no default


> Could anyone think of a case where the current behavior is desirable ?
>   

well, one scenario might be that one wants to print a collection of
items with an arbitrary format, supplied by the users, e.g.

    foo = function(fmt) {
       a = ...
       b = ...
       ...
       s = sprintf(fmt, a, b, ...)
       ... }

without having to examine the format to establish which values are
needed.  in the current state, sprintf would use those it would need to
use with a particular format, with no undesirable complaints.


>   
>> but they *are* evaluated nevertheless, e.g.:
>>
>>    sprintf('%d', 0, {print(1)})
>>    # "1"
>>    # [1] "0"
>>
>> it might be a good idea to document this behaviour.
>>     
>
> actually I think it should be changed to be more strict (see above).
>   

strict in which sense?  enforce a constraint on the number of arguments
to that needed by a specific format?  or do you mean evaluation of only
those arguments that are needed in a format?  or both?

what about:

    sprintf('%2$s', {print(1)}, 2)
    # too many arguments?
    # should 1 be printed?

    sprintf('%2$s', , 2)
    # too few arguments?
    # missing value?  (yes, sprintf is .Internal, but...)


> Thank you for the constructive feedback!
>   

not much to thank for...  certainly, it's the first time my feedback is
called 'constructive'.  i'm making progress, am i not?

btw., thank you for the fixes.  i appreciate your efforts a lot.

best,
vQ