Re: [PATCH] Analysis of bug in BSD patch

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 05/23/13 06:07, Stefan Esser wrote:
> 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.

Sounds good to me, thanks for working on this!

Cheers,
- -- 
Xin LI <delphij_at_delphij.net>    https://www.delphij.net/
FreeBSD - The Power to Serve!           Live free or die
-----BEGIN PGP SIGNATURE-----

iQEcBAEBCgAGBQJRnk4XAAoJEG80Jeu8UPuzOsUH/jw0nYAL7HzUw9diyOJb9uJb
if4IZeVQzqwd66gVQGg4PD/ZRbuTlLNugA+ljb+oIEB6P5i4psx4ki0QNZbjmhgS
Ft4UnyeaOYe4IxpevvO5Tzq0LbUVLS2fnzPzHhkv2aCmddALpQ5sPJgLpMQZ/VCa
WAjPY9CZhu3aWXLCOVAH8KlL4crwMEVlgnL+onP4eqZydddvP5t058otvggZqHVL
oNgzMYanT6BANQlUD9B/bnnLK7kTdIvBSB5hEd4l8oIa2zMrEjWrnC+5K/WlbarU
yXdCiixnBrsrkaECrXZPJE6ImzOxw7f6GKOJ4cyJ2fUzJ9mwq8Oe84VGGiI+Rbw=
=Kkia
-----END PGP SIGNATURE-----