[MPlayer-dev-eng] [PATCH 2/4] String handling audit/cleanup take 2

[Reimar Döffinger]

Hello,
On Wed, Mar 07, 2007 at 02:58:11AM -0500, Rich Felker wrote:
> I understand this sentiment, and that there have been bugs (or at
> least unintended off-by-one type behavior changes) in some of the
> patches. However I'm fairly unhappy with how this whole thread has
> been handled. Auditing insecure string behavior is a very difficult
> task, and when someone steps up and does it, rather than being
> thankful, everyone gets defensive and starts dwelling on minor bugs
> and complaining that changes are not needed.

These are huge patches, please bear with me when my replies are rather
short and not very friendly then.
And I sure don't mind if you review and apply some of these patches,
esp. if it's not my code I won't flame anyone because of such changes.

> To those complaining, let me say: If you're willing and capable of
> doing a complete audit of MPlayer, then by all means let the strncpy
> and sprintf crap remain in place and do it. Otherwise, removing the
> use of these bad functions is quite a useful service to the project,
> in that it makes auditing easier.

If you replace a sprintf by an snprintf and the following code assumes
that the *printf reproduced the full string then you got no closer to a
proper audit.
So, and to be productive I was thinking of something like that (I did
not think long about it, so bear with any bugs/stupidity and ignore the
stupid names):
add a
mpsnprintf that behaves like snprintf except that it aborts when the
string does not fit
BUFPRINTF macro for static arrays that does mpsnprintf(buf, sizeof(buf), ...)

In some ways this also applies to strncpy/strlcpy, but those changes
are desirable for other reasons anyway, so I am of course less critical.

Greetings,
Reimar Döffinger