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
