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

[maechler at stat.math.ethz.ch]

>>>>> "vQ" == Waclaw Marcin Kusnierczyk <Waclaw.Marcin.Kusnierczyk at idi.ntnu.no>
>>>>>     on Sat, 25 Apr 2009 19:40:27 +0200 (CEST) writes:

    vQ> Martin Maechler wrote:
    >> 
    MM> well, it is basically (+ a few bytes ?)  the same 8192
    MM> 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)?
    >>> 
    >> 


    >>> 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.
    >> 

    vQ> yes, but

    vQ>     sptinf('%q%s', 1)


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

yes, but only the error message somewhat suggests that;
at the moment, I'd like to keep it, since really the user
*should* think of using the number formats, rather than %s
{which just calls  as.character(.)}  for numeric arguments


    >> Thank you for the suggestion!
    >> 

  vQ> 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),
    >>> 
    >> 

  MM> I think we should signal an error if there are too many arguments.

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

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

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

very good point; thanks!

    vQ> Interestingly,

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

yes, empty (aka "missing") arguments are not allowed in  sprintf().

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

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

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

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

ok. you have given good examples which make me revert my
proposal, i.e. continue to not erroring about "too many" arguments. 

    >>> 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.

   MM> actually I think it should be changed to be more strict
   MM> (see above).

as a matter of fact, and the result of many more examples,
I've changed my oppinion and now agree with your original
proposal:

I've just commmited another sprintf() patch which (among more
more important changes) *documents* that all arguments of
sprintf() are evaluated; this actually already entails that
empty / missing arguments are not allowed.

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

    vQ> what about:

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

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


    >> Thank you for the constructive feedback!
    >> 

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

indeed!  :-)

Martin Maechler, ETH Zurich

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

    vQ> best, vQ