[FFmpeg-devel] [PATCH] bprint: Remove custom vsnprintf

[Ronald S. Bultje]

Hi,

On Sep 9, 2012 7:46 PM, "Nicolas George" <nicolas.george at normalesup.org>
wrote:
>
> Le quartidi 24 fructidor, an CCXX, Ronald S. Bultje a écrit :
> > I doubt that introducing a hardcoded upper bound on the return value
> > of your vsnprintf replacement is a great replacement for this
> > (admittedly) buggy behaviour.
> >
> > (Not that I have a better idea, I'm just saying that from a
> > design-point-of-view, swapping one bug for another when introducing a
> > completely new API isn't great.)
>
> That is true, but that is the best idea I could find, short of grabbing a
> whole printf implementation from some libc (and that is large).
>
> The fact that the docs from microsoft are unreliable does not help: the
> version _vscprintf is perfect, according to MSDN, but it is not compatible
> with older windows, and that is not in the MSDN.
>
> Personally, I do not care about windows, let alone versions that are more
> than a decade obsolete and unsupported by their own editor. Since this is
> code that I wrote, I feel obligated to maintain it for those systems if
> people care about them (and since the question was raised on the
> mailing-list, obviously there are people who care), but I would be very
> happy to concede the burden to someone else.
>
> Currently, here are the pros and cons of the various solutions:
>
> * Using _vscprintf: completely correct, but does not wok with w2k.
>
> * Using the version you wrote for compat/: does not allow bprint to
enlarge
>   its buffer as necessary, nor to compute the real size of the string if
it
>   grows larger than the buffer (that is the documented behaviour).
>
> * Doubling the bprint buffer each time: if the user has set a limit on the
>   buffer size, bprint can not count the size of the string beyond that.
>
> * Using vsnprintf in a large local buffer: compatible with older windows,
>   but produces invalid results if a single print operation is larger than
>   the local buffer. I believe this is acceptable, as bprint is made for
>   small prints that accumulate into something possibly large, but this is
>   not 100% satisfactory, far from it.
>
> * Using a large local buffer, doubling its size if it is too small: not as
>   lightweight as intendes (but it is windows...), and a risk of infinite
>   loop / memory consumption if vsnprintf fails for a reason différent
than a
>   buffer too small (is it possible? with a "%ls" format string and
>   incompatible encodings maybe? is it possible to test it?).
>
> IMHO, after thinking about it some more, I believe the more satisfactory
> solution would be to keep the version with _vscprintf (maybe merging it
with
> your implementation in compat/)

I think I agree.

(Fallbacks for older systems can be crappy; the real solution is to
upgrade.)

Ronald