I have installed a modified version of the patch
(1dcdf3de8e27cc130968891ee5a529a461a248da), updated in the ways you
suggested.
On Sat, Nov 2, 2024 at 9:24 AM Bernhard Voelker
<m...@bernhard-voelker.de> wrote:
>
> Hi James,
>
> thanks, technically this patch fixes it, yet I think it deserves some more
> love before pushing.
>
> First of all, this patch does not cleanly apply.
>
> $ git am /tmp/x.patch
> Applying: Fix Savannah bug 66365.
> error: git diff header lacks filename information when removing 1 leading
> pathname component (line 82)
> Patch failed at 0001 Fix Savannah bug 66365.
> ...
>
> Didn't you use 'git format-patch -1' and 'git send-email ...'?
>
> On 10/31/24 17:28, James Youngman wrote:
> > From e7641882e3ef373b6c58bcf27fbc49269e2541f3 Mon Sep 17 00:00:00 2001
> > From: James Youngman <ja...@youngman.org>
> > Date: Thu, 31 Oct 2024 15:06:02 +0000
> > Subject: [PATCH] [find] Fix Savannah bug 66365.
>
> the bug should be mentioned in the commit message, but the Subject line
> should better tell about the change, e.g.
>
> find: only treat '+' special after pure '{}' in -exec option family
>
> > A "+" only terminates -exec when it immediately follows an argument
> > which is exactly "{}".
>
> Let's also explain the change and reasoning for it in a NEWS entry.
>
> > ---
> > find/parser.c | 21 ++++++++++++-------
> [...]
> > create mode 100644 find/testsuite/find.posix/sv-bug-66365-exec.exp
> > create mode 100644 find/testsuite/find.posix/sv-bug-66365-exec.xo
>
> I'd prefer going with the newer style for the tests like
> 'tests/find/exec-plus-last-file.sh'.
>
> > diff --git a/find/parser.c b/find/parser.c
> > index ad3b9904..d7b07c40 100644
> > --- a/find/parser.c
> > +++ b/find/parser.c
> > @@ -2770,7 +2770,7 @@ insert_exec_ok (const char *action,
> > {
> > int start, end; /* Indexes in ARGV of start & end of cmd.
> > */
> > int i; /* Index into cmd args */
> > - int saw_braces; /* True if previous arg was '{}'. */
> > + bool plus_is_terminator; /* True if previous arg was '{}'. */
>
> I like the change to bool, but I'm not sure whether the new name is clearer
> here.
> Actually it should say "the previous argument were pure braces only".
> Maybe 'saw_pure_braces'?
>
> > bool allow_plus; /* True if + is a valid terminator */
> > int brace_count; /* Number of instances of {}. */
> > const char *brace_arg; /* Which arg did {} appear in? */
> > @@ -2827,24 +2827,31 @@ insert_exec_ok (const char *action,
> > * Also figure out if the command is terminated by ";" or by "+".
> > */
> > start = *arg_ptr;
> > - for (end = start, saw_braces=0, brace_count=0, brace_arg=NULL;
> > + for (end = start, plus_is_terminator=false, brace_count=0,
> > brace_arg=NULL;
> > (argv[end] != NULL)
> > && ((argv[end][0] != ';') || (argv[end][1] != '\0'));
> > end++)
> > {
> > /* For -exec and -execdir, "{} +" can terminate the command. */
> > - if ( allow_plus
> > - && argv[end][0] == '+' && argv[end][1] == 0
> > - && saw_braces)
> > + if (allow_plus && plus_is_terminator
> > + && argv[end][0] == '+' && argv[end][1] == 0)
> > {
> > our_pred->args.exec_vec.multiple = 1;
> > break;
> > }
> >
> > - saw_braces = 0;
> > + plus_is_terminator = false;
> > if (mbsstr (argv[end], "{}"))
> > {
> > - saw_braces = 1;
> > + if (0 == strcmp(argv[end], "{}"))
> > + {
> > + /* Savannah bug 66365: + only terminates the predicate
> > + * immediately after an argument which is exactly, "{}".
> > + * However, the "{}" in "x{}" should get expanded for
> > + * the ";" case.
> > + */
> > + plus_is_terminator = true;
> > + }
>
> The indentation is odd.
>
> > brace_arg = argv[end];
> > ++brace_count;
> >
>
> BTW: the guard to prevent '-exec {} ;' seems to be ineffective for a while as
> well.
> I'll check.
>
> Have a nice day,
> Berny
