[patch] Process find's leading options in a POSIX-compliant way

From c10a0126dd56e42a8118494cf67a6c74eb58c0db Mon Sep 17 00:00:00 2001
From: James Youngman <[email protected]>
Date: Mon, 3 Jun 2024 08:44:12 +0100
Subject: [PATCH] find: Process -P -H -L options in a POSIX-compliant way.
To: [email protected]

* tests/find/posix-options.sh: New test for option combinations like
find -LP which previously were not recognised.
* tests/local.mk(sh_tests): add tests/find/posix-options.sh.
* find/util.c(process_leading_options): process leading options with
getopt (instead of strcmp) for greater POSIX compliance
* NEWS: mention this bugfix.
---
 NEWS                        |   4 ++
 find/util.c                 | 106 ++++++++++++++++++++++--------------
 tests/find/posix-options.sh |  97 +++++++++++++++++++++++++++++++++
 tests/local.mk              |   1 +
 4 files changed, 167 insertions(+), 41 deletions(-)
 create mode 100755 tests/find/posix-options.sh

diff --git a/NEWS b/NEWS
index e2a8e067..40fd0309 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,10 @@ GNU findutils NEWS - User visible changes.      -*- outline -*- (allout)
 
 * Noteworthy changes in release ?.? (????-??-??) [?]
 
+** Bug Fixes
+
+  Find now accepts combinations of leading options, for example "find
+  -PP".  Previously, these were incorrectly rejected.
 
 * Noteworthy changes in release 4.10.0 (2024-06-01) [stable]
 
diff --git a/find/util.c b/find/util.c
index a1736807..709e2518 100644
--- a/find/util.c
+++ b/find/util.c
@@ -25,6 +25,7 @@
 #include <fcntl.h>
 #include <limits.h>
 #include <string.h>
+#include <unistd.h>   /* for getopt() */
 #include <sys/stat.h> /* for fstatat() */
 #include <sys/time.h>
 #include <sys/utsname.h>
@@ -908,56 +909,79 @@ process_optimisation_option (const char *arg)
 int
 process_leading_options (int argc, char *argv[])
 {
-  int i, end_of_leading_options;
+  bool done = false;
+  int opt;
 
-  for (i=1; (end_of_leading_options = i) < argc; ++i)
+  /* We don't want getopt to print its own error messages. */
+  opterr = 0;
+
+  /* We put + as the first character of optstring in order to prevent
+   * getopt permuting the order of the options.  Therefore we require
+   * GNU getopt, since this is an extension.  We use ':' as the second
+   * character to change the way getopt handles missing
+   * option-arguments (as specified by POSIX).
+   */
+  while (!done && (opt = getopt(argc, argv, "+:HLPD:O:")) != -1)
     {
-      if (0 == strcmp ("-H", argv[i]))
-        {
+      switch (opt)
+	{
+	case 'H':
           /* Meaning: dereference symbolic links on command line, but nowhere else. */
           set_follow_state (SYMLINK_DEREF_ARGSONLY);
-        }
-      else if (0 == strcmp ("-L", argv[i]))
-        {
+	  break;
+
+	case 'L':
           /* Meaning: dereference all symbolic links. */
           set_follow_state (SYMLINK_ALWAYS_DEREF);
-        }
-      else if (0 == strcmp ("-P", argv[i]))
-        {
+	  break;
+
+	case 'P':
           /* Meaning: never dereference symbolic links (default). */
           set_follow_state (SYMLINK_NEVER_DEREF);
-        }
-      else if (0 == strcmp ("--", argv[i]))
-        {
-          /* -- signifies the end of options. */
-          end_of_leading_options = i+1; /* Next time start with the next option */
-          break;
-        }
-      else if (0 == strcmp ("-D", argv[i]))
-        {
-          if (argc <= i+1)
-            {
-              error (0, 0, _("Missing argument after the -D option."));
-              usage (EXIT_FAILURE);
-            }
-          process_debug_options (argv[i+1]);
-          ++i;                  /* skip the argument too. */
-        }
-      else if (0 == strncmp ("-O", argv[i], 2))
-        {
-          process_optimisation_option (argv[i]+2);
-        }
-      else
-        {
-          /* Hmm, must be one of
-           * (a) A path name
-           * (b) A predicate
-           */
-          end_of_leading_options = i; /* Next time start with this option */
-          break;
-        }
+	  break;
+
+	case 'D':
+          process_debug_options (optarg);
+	  break;
+
+	case 'O':
+          process_optimisation_option (optarg);
+	  break;
+
+	case ':':
+	  /* There is a missing option-argument.  We use this switch
+	   * statement below to ensure that the existing translations
+	   * for the error messages are used.  Once translations have
+	   * caught up and include the error message which uses %c, we
+	   * can remove the special cases for -E and -D and use only
+	   * the generic error message.
+	   */
+	  switch (optopt)
+	    {
+	    case 'E':
+	      error (1, 0, _("Missing argument after the -E option."));
+	      break;
+	    case 'D':
+	      error (1, 0, _("Missing argument after the -D option."));
+	      break;
+	    default:
+	      /* TRANSLATORS: At the moment this case is not actually
+	       * reached but it is included so that this message gets
+	       * translated.  Eventually this will be the only
+	       * message.  See the comment above for an explanation.
+	       */
+	      error (1, 0, _("Missing argument after the -%c option."), optopt);
+	    }
+	  break;
+
+	case '?':
+	default:
+	  /* This is the first argument that isn't a leading option. */
+	  done = true;
+	  break;
+	}
     }
-  return end_of_leading_options;
+  return optind;
 }
 
 static struct timespec
diff --git a/tests/find/posix-options.sh b/tests/find/posix-options.sh
new file mode 100755
index 00000000..055dab58
--- /dev/null
+++ b/tests/find/posix-options.sh
@@ -0,0 +1,97 @@
+#!/bin/sh
+# Verify that -P, -L, -H are processed in a POSIX-compliant way.
+
+# Copyright (C) 2024 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/tests/init.sh"; fu_path_prepend_
+print_ver_ find
+
+# create test files.
+touch empty
+ln -s empty symlink
+ln -s does-not-exist broken-symlink
+
+# Verify that we don't follow symbolic links by default.
+echo "== testing: find with no options"
+echo symlink >| expected
+find symlink -type l >| out
+compare expected out || { fail=1; cat out; }
+
+# Verify that we don't follow symbolic links with -P.
+echo "== testing: find -P"
+echo symlink >| expected
+find -P symlink -type l >| out
+compare expected out || { fail=1; cat out; }
+
+# Verify that we accept -PP
+echo "== testing: find -PP"
+echo symlink >| expected
+find -PP symlink -type l >| out
+compare expected out || { fail=1; cat out; }
+
+# Verify that with -L we resolve the symlink.
+echo "== testing: find -L"
+# Expect no output since -L doesn't think symlink is a symlink
+# (because it gets resolved).
+true >| expected
+find -L symlink -type l >| out
+compare expected out || { fail=1; cat out; }
+
+# Verify that -LL is accepted.
+echo "== testing: find -LL"
+# Expect no output since -L doesn't think symlink is a symlink
+# (because it gets resolved).
+true >| expected
+find -LL symlink -type l >| out
+compare expected out || { fail=1; cat out; }
+
+# Verify that with -L a broken symlink still looks like a symlink.
+echo "== testing: find -L with broken symlink"
+echo broken-symlink >| expected
+find -L broken-symlink -type l >| out
+compare expected out || { fail=1; cat out; }
+
+# Verify that -LP is erquivalent to -P
+echo "== testing: find -LP"
+echo symlink >| expected
+find -P symlink -type l >| out
+compare expected out || { fail=1; cat out; }
+
+# Verify that -H resolves symlinks on the command line.
+echo "== testing: find -H"
+# Expect no output since -H doesn't think symlink is a symlink
+# (because it gets resolved).
+true >| expected
+find -H symlink -type l >| out
+compare expected out || { fail=1; cat out; }
+
+# Verify that -PH is equivalent to -H
+echo "== testing: find -PH"
+# Expect no output since -H doesn't think symlink is a symlink
+# (because it gets resolved).
+true >| expected
+find -PH symlink -type l >| out
+compare expected out || { fail=1; cat out; }
+
+# Verify that -LH is equivalent to -H
+echo "== testing: find -LH"
+# Expect no output since -H doesn't think symlink is a symlink
+# (because it gets resolved).
+true >| expected
+find -LH symlink -type l >| out
+compare expected out || { fail=1; cat out; }
+
+Exit $fail
diff --git a/tests/local.mk b/tests/local.mk
index 1fe14c01..cfe2a661 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -127,6 +127,7 @@ sh_tests = \
   tests/find/used.sh \
   tests/find/newer.sh \
   tests/find/opt-numeric-arg.sh \
+  tests/find/posix-options.sh \
   tests/find/user-group-max.sh \
   tests/xargs/conflicting_opts.sh \
   tests/xargs/verbose-quote.sh \
-- 
2.39.2