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

Nicholas Kain njkain at gmail.com
Fri Mar 2 22:38:48 CET 2007


On 3/2/07, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de> wrote:
> 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.

Yep, I didn't like that code either, which is why I converted it to
strlcat and strlcpy in the original version.  I've converted back.
That was the only mechanical conversion in the patchset.

Either way, reviewing the patchset for possible errors will lead to
more code examination, which is only beneficial to an audit.


More information about the MPlayer-dev-eng mailing list