Hi,
On Tue, 20 Feb 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>
> > Ha, I did it by
> >
> > $ perl -pi.bup -e \
> > 's/strncmp\( *("[^"]*"), *([^"]*), *[0-9]* *\)/prefixcmp\($2, $1\)/g' \
> > $(git ls-files)
> >
> > and
> >
> > $ perl -pi.bup -e \
> > 's/strncmp\( *([^"]*), *("[^"]*"), *[0-9]* *\)/prefixcmp\($1, $2\)/g' \
> > $(git-ls-files)
> >
> > Of course, I missed the two ,ofs ones, but a git grep -n strncmp brought
> > these up.
>
> I think you totally missed my point. I wanted to make sure that
> things like these do not go unnoticed:
>
> if (!strncmp(arg, "--foo==", 6))
> if (strncmp(line, "foo\nbar", 8))
>
> Both are probably incorrectly written code in the original, but
> probably would _happen_ to be working (for a certain definition
> of "working" -- the former probably wanted to make sure the
> parameter is of form "--foo=something", and the latter wanted to
> check the line has the 7 bytes terminated with NUL). But your
> conversion would make them actually start behaving incorrectly.
>
> And the worst part of this is that the change that caused to
> expose these bugs would be literally _buried_ in 1800 lines of
> "mechanical conversion" patch which is mind-numbing to audit.
>
> That's why you are better off writing mechanical conversion
> script in stricter than seemingly necessary to catch only the
> safe conversion target, while accepting false negatives.
All true. I thought fixing them without checking was fine, but you are
right: better safe than sorry.
Ciao,
Dscho
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html