[PATCH] Analysis of bug in BSD patch (was: Re: Bug in BSD patch)

Am 23.05.2013 13:02, schrieb Stefan Esser:
> This appears to be a problem with the new BSD patch in -CURRENT:
> 
> # gnupatch -d /usr/ports/textproc/texi2html/work/texi2html-5.0 -E -p0 \
>   -V simple -C < files/patch-texi2html.pl
> Hmm...  Looks like a unified diff to me...
> The text leading up to this was:
> --------------------------
> |--- texi2html.pl       2012-07-09 10:54:41.000000000 +0200
> |+++ /usr/local/bin/texi2html   2012-07-09 10:53:16.000000000 +0200
> --------------------------
> Patching file texi2html.pl using Plan A...
> Hunk #1 succeeded at 1933.
> done
> 
> # bsdpatch -d /usr/ports/textproc/texi2html/work/texi2html-5.0 -E -p0 \
>   -V simple -C < files/patch-texi2html.pl
> Hmm...  Looks like a unified diff to me...
> The text leading up to this was:
> --------------------------
> |--- texi2html.pl       2012-07-09 10:54:41.000000000 +0200
> |+++ /usr/local/bin/texi2html   2012-07-09 10:53:16.000000000 +0200
> --------------------------
> Patching file /usr/local/bin/texi2html using Plan A...
> Reversed (or previously applied) patch detected!  Assume -R? [y]
> 
> Obviously, BSD patch does not select the file with the shortest
> path name as the target. I have not checked the source code, but
> you should definitely open a PR for this bug!

Further information: The implemented logic does not follow the
logic explained in the man page. Instead of using the file with
the least order of path name components, shortest filename and
finally shortest basename (with the search stopping as soon as
one of these conditions is true), it uses the first file name
to check as the reference, and only chooses another file name if
*all* of the above comparisons are in favour of the latter file.

This is wrong, because files with less components will only be
considered, if both of the other conditions are true as well.
In fact, the first filename to be checked has good chances to
be selected in the end, since it only needs to be better with
regard to any one of the criteria ...

The following patch fixes the behaviour and makes it compliant
with both the man page and GNU patch:

Index: pch.c
===================================================================
--- pch.c	(revision 250926)
+++ pch.c	(working copy)
_at__at_ -1537,10 +1537,16 _at__at_
 			continue;
 		if ((tmp = num_components(names[i].path)) > min_components)
 			continue;
-		min_components = tmp;
+		if (tmp < min_components) {
+			min_components = tmp;
+			best = names[i].path;
+		}
 		if ((tmp = strlen(basename(names[i].path))) > min_baselen)
 			continue;
-		min_baselen = tmp;
+		if (tmp < min_baselen) {
+			min_baselen = tmp;
+			best = names[i].path;
+		}
 		if ((tmp = strlen(names[i].path)) > min_len)
 			continue;
 		min_len = tmp;

Please review this patch - I'd like to commit it to HEAD if there
are no objections.

Regards, STefan