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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.



-
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]