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

[Reimar Döffinger]

Hello,
On Fri, Mar 02, 2007 at 04:15:20PM -0500, Rich Felker wrote:
> On Fri, Mar 02, 2007 at 04:04:23PM -0500, Nicholas Kain wrote:
> > On 3/2/07, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de> 
[...]
> > >Most of the sprintf->snprintf changes are either pointless or just plain
> > >wrong (esp. in asxparser).
> 
> Hm? I didn't see any that were wrong. If nothing else they have a
> point in that they save someone doing a "grep sprintf" audit the
> trouble of having to look at these lines again.

Well, not more "wrong" than the original code, but in asxparser the
destination string pointer is increased but len is not.
Using snprintf mechanically IMO is even worse than sprintf, it only
creates an illusion of safety where there is no reason for it - e.g.
because the limit is incorrect, or because the string being terminated
at some random position is critical. E.g. in asxparser the needed length
for the string is calculated - though it might miss a check against an
integer overflow. If using snprintf, that calculation probably should
just be removed.
That's why I don't like the snprintf changes mixed in, properly
reviewing them will take a looong time.

Greetings,
Reimar Döffinger