Re: [PATCH 2/4] Mechanical conversion to use prefixcmp()

From: Junio C Hamano <junkio@cox.net>
Date: Feb 21, 2007, 06:43 UTC
Message-ID: <7v7iuct3ic.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <Pine.LNX.4.63.0702201243000.22628@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 14 quoted lines

> 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.