Date: Thu, 23 May 2013 10:12:56 -0700 From: Xin Li <delphij@delphij.net> To: Stefan Esser <se@freebsd.org> Cc: se@localhost.FreeBSD.org, freebsd-current@freebsd.org, delphij@freebsd.org, gabor@freebsd.org Subject: Re: [PATCH] Analysis of bug in BSD patch Message-ID: <519E4E18.1060605@delphij.net> In-Reply-To: <20130523130751.GA53009@StefanEsser.freebsd.org> References: <519DBC27.9030600@gmx.com> <519DF73E.40307@freebsd.org> <20130523130751.GA53009@StefanEsser.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
-----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) @@ -1537,10 +1537,16 @@ 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@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-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?519E4E18.1060605>