Re: [PATCH 5/5] posix: Sync fnmatch with gnulib

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 730 bytes --]

On 12/30/20 12:15 PM, Adhemerval Zanella wrote:

> -  ssize_t level;
> +  size_t level;

'level' should be ptrdiff_t not ssize_t, for portability to 
(now-ancient, but still allowed by POSIX) hosts where ssize_t is 32 bits 
and size_t is 64 bits.

> -    CHAR str[];
> +    CHAR str[FLEXIBLE_ARRAY_MEMBER];

This assumes C99 flex array members which is fine for glibc but dubious 
for Gnulib; it should be safer to use __flexarr.

> Because otherwise it triggers some glibc regressions:

Thanks for spotting that. I installed the attached patch into Gnulib, 
which should fix the glibc regressions and the other abovementioned 
glitches, so that you should now be able to sync fnmatch from Gnulib 
unchanged.

[-- Attachment #2: 0002-fnmatch-merge-from-glibc-proposal.patch --]
[-- Type: text/x-patch, Size: 3575 bytes --]

From a7ad4b110fd6b7dde424dceb46c9c09c31cfbe69 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 31 Dec 2020 13:35:53 -0800
Subject: [PATCH 2/2] fnmatch: merge from glibc + proposal

This merges the change proposed by Adhemerval Zanella in:
https://sourceware.org/pipermail/libc-alpha/2020-December/121212.html
which fixes a Gnulib bug that led to a failed assert.
* lib/fnmatch_loop.c (EXT): Use signed level, not unsigned, and
check that it stays nonnegative.  Use __flexarr instead of
FLEXIBLE_ARRAY_MEMBER, to port better to glibc.
* tests/test-fnmatch.c (main): New test cases, taken from glibc.
---
 ChangeLog            |  9 +++++++++
 lib/fnmatch_loop.c   |  7 +++----
 tests/test-fnmatch.c | 10 ++++++++++
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 5da7c043a..661a1ee94 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2020-12-31  Paul Eggert  <eggert@cs.ucla.edu>
 
+	fnmatch: merge from glibc + proposal
+	This merges the change proposed by Adhemerval Zanella in:
+	https://sourceware.org/pipermail/libc-alpha/2020-December/121212.html
+	which fixes a Gnulib bug that led to a failed assert.
+	* lib/fnmatch_loop.c (EXT): Use signed level, not unsigned, and
+	check that it stays nonnegative.  Use __flexarr instead of
+	FLEXIBLE_ARRAY_MEMBER, to port better to glibc.
+	* tests/test-fnmatch.c (main): New test cases, taken from glibc.
+
 	glob: merge proposed glibc changes
 	This merges the change proposed by Adhemerval Zanella in:
 	https://sourceware.org/pipermail/libc-alpha/2020-December/121211.html
diff --git a/lib/fnmatch_loop.c b/lib/fnmatch_loop.c
index c533107a2..e5dac38b4 100644
--- a/lib/fnmatch_loop.c
+++ b/lib/fnmatch_loop.c
@@ -978,12 +978,12 @@ EXT (INT opt, const CHAR *pattern, const CHAR *string, const CHAR *string_end,
      bool no_leading_period, int flags, size_t alloca_used)
 {
   const CHAR *startp;
-  size_t level;
+  ptrdiff_t level;
   struct patternlist
   {
     struct patternlist *next;
     CHAR malloced;
-    CHAR str[FLEXIBLE_ARRAY_MEMBER];
+    CHAR str __flexarr;
   } *list = NULL;
   struct patternlist **lastp = &list;
   size_t pattern_len = STRLEN (pattern);
@@ -994,7 +994,7 @@ EXT (INT opt, const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 
   /* Parse the pattern.  Store the individual parts in the list.  */
   level = 0;
-  for (startp = p = pattern + 1; ; ++p)
+  for (startp = p = pattern + 1; level >= 0; ++p)
     if (*p == L_('\0'))
       {
         /* This is an invalid pattern.  */
@@ -1065,7 +1065,6 @@ EXT (INT opt, const CHAR *pattern, const CHAR *string, const CHAR *string_end,
             *lastp = newp;                                                    \
             lastp = &newp->next
             NEW_PATTERN;
-            break;
           }
       }
     else if (*p == L_('|'))
diff --git a/tests/test-fnmatch.c b/tests/test-fnmatch.c
index a094c1fa7..1d58689cf 100644
--- a/tests/test-fnmatch.c
+++ b/tests/test-fnmatch.c
@@ -52,5 +52,15 @@ main ()
    */
   ASSERT (res = fnmatch ("[/b", "[/b", 0) == 0);
 
+  ASSERT (fnmatch ("[[:alpha:]'[:alpha:]\0]", "a", 0) == FNM_NOMATCH);
+  ASSERT (fnmatch ("[a[.\0.]]", "a", 0) == FNM_NOMATCH);
+#ifdef FNM_EXTMATCH
+  ASSERT (fnmatch ("**(!()", "**(!()", FNM_EXTMATCH) == 0);
+#endif
+#ifdef FNM_LEADING_DIR
+  ASSERT (fnmatch ("x?y", "x/y/z", FNM_PATHNAME | FNM_LEADING_DIR)
+          == FNM_NOMATCH);
+#endif
+
   return 0;
 }
-- 
2.27.0