* [PATCH 1/2] posix: Falling back to non wide mode in case of encoding error [BZ #14185]
@ 2021-02-02 13:08 Adhemerval Zanella
2021-02-02 13:08 ` [PATCH 2/2] posix: Remove alloca usage for internal fnmatch implementation Adhemerval Zanella
2021-02-22 10:54 ` [PATCH 1/2] posix: Falling back to non wide mode in case of encoding error [BZ #14185] Florian Weimer
0 siblings, 2 replies; 4+ messages in thread
From: Adhemerval Zanella @ 2021-02-02 13:08 UTC (permalink / raw)
To: libc-alpha; +Cc: bug-gnulib, Paul Eggert
Gnulib has added the proposed fix with aed23714d60 (done in 2005), but
recently with a glibc merge with 67306f6 (done in 2020 with sync back)
it has fallback to old semantic to return -1 on in case of failure.
From gnulib developer feedback it was an oversight. Although the full
fix for BZ #14185 would require to rewrite fnmatch implementation to use
mbrtowc instead of mbsrtowcs on the full input, this mitigate the issue
and it has been used by gnulib for a long time.
This patch also removes the alloca usage on the string convertion to
wide characters before calling the internal function.
Checked on x86_64-linux-gnu.
---
posix/fnmatch.c | 160 ++++++++++++++--------------------------
posix/tst-fnmatch.input | 2 +
2 files changed, 58 insertions(+), 104 deletions(-)
diff --git a/posix/fnmatch.c b/posix/fnmatch.c
index b8a71f164d..a66c9196c7 100644
--- a/posix/fnmatch.c
+++ b/posix/fnmatch.c
@@ -75,6 +75,7 @@ extern int fnmatch (const char *pattern, const char *string, int flags);
#include <intprops.h>
#include <flexmember.h>
+#include <scratch_buffer.h>
#ifdef _LIBC
typedef ptrdiff_t idx_t;
@@ -231,121 +232,72 @@ is_char_class (const wchar_t *wcs)
#include "fnmatch_loop.c"
+static int
+fnmatch_convert_to_wide (const char *str, struct scratch_buffer *buf,
+ size_t *n)
+{
+ mbstate_t ps;
+ memset (&ps, '\0', sizeof (ps));
+
+ size_t nw = buf->length / sizeof (wchar_t);
+ *n = strnlen (str, nw - 1);
+ if (__glibc_likely (*n < nw))
+ {
+ const char *p = str;
+ *n = mbsrtowcs (buf->data, &p, *n + 1, &ps);
+ if (__glibc_unlikely (*n == (size_t) -1))
+ /* Something wrong.
+ XXX Do we have to set 'errno' to something which mbsrtows hasn't
+ already done? */
+ return -1;
+ if (p == NULL)
+ return 0;
+ memset (&ps, '\0', sizeof (ps));
+ }
+
+ *n = mbsrtowcs (NULL, &str, 0, &ps);
+ if (__glibc_unlikely (*n == (size_t) -1))
+ return -1;
+ if (!scratch_buffer_set_array_size (buf, *n + 1, sizeof (wchar_t)))
+ {
+ __set_errno (ENOMEM);
+ return -2;
+ }
+ assert (mbsinit (&ps));
+ mbsrtowcs (buf->data, &str, *n + 1, &ps);
+ return 0;
+}
int
fnmatch (const char *pattern, const char *string, int flags)
{
if (__glibc_unlikely (MB_CUR_MAX != 1))
{
- mbstate_t ps;
size_t n;
- const char *p;
- wchar_t *wpattern_malloc = NULL;
- wchar_t *wpattern;
- wchar_t *wstring_malloc = NULL;
- wchar_t *wstring;
- size_t alloca_used = 0;
-
- /* Convert the strings into wide characters. */
- memset (&ps, '\0', sizeof (ps));
- p = pattern;
- n = strnlen (pattern, 1024);
- if (__glibc_likely (n < 1024))
+ struct scratch_buffer wpattern;
+ scratch_buffer_init (&wpattern);
+ struct scratch_buffer wstring;
+ scratch_buffer_init (&wstring);
+ int r;
+
+ /* Convert the strings into wide characters. Any conversion issue
+ fallback to the ascii version. */
+ r = fnmatch_convert_to_wide (pattern, &wpattern, &n);
+ if (r == 0)
{
- wpattern = (wchar_t *) alloca_account ((n + 1) * sizeof (wchar_t),
- alloca_used);
- n = mbsrtowcs (wpattern, &p, n + 1, &ps);
- if (__glibc_unlikely (n == (size_t) -1))
- /* Something wrong.
- XXX Do we have to set 'errno' to something which mbsrtows hasn't
- already done? */
- return -1;
- if (p)
- {
- memset (&ps, '\0', sizeof (ps));
- goto prepare_wpattern;
- }
- }
- else
- {
- prepare_wpattern:
- n = mbsrtowcs (NULL, &pattern, 0, &ps);
- if (__glibc_unlikely (n == (size_t) -1))
- /* Something wrong.
- XXX Do we have to set 'errno' to something which mbsrtows hasn't
- already done? */
- return -1;
- if (__glibc_unlikely (n >= (size_t) -1 / sizeof (wchar_t)))
- {
- __set_errno (ENOMEM);
- return -2;
- }
- wpattern_malloc = wpattern
- = (wchar_t *) malloc ((n + 1) * sizeof (wchar_t));
- assert (mbsinit (&ps));
- if (wpattern == NULL)
- return -2;
- (void) mbsrtowcs (wpattern, &pattern, n + 1, &ps);
- }
-
- assert (mbsinit (&ps));
- n = strnlen (string, 1024);
- p = string;
- if (__glibc_likely (n < 1024))
- {
- wstring = (wchar_t *) alloca_account ((n + 1) * sizeof (wchar_t),
- alloca_used);
- n = mbsrtowcs (wstring, &p, n + 1, &ps);
- if (__glibc_unlikely (n == (size_t) -1))
- {
- /* Something wrong.
- XXX Do we have to set 'errno' to something which
- mbsrtows hasn't already done? */
- free_return:
- free (wpattern_malloc);
- return -1;
- }
- if (p)
- {
- memset (&ps, '\0', sizeof (ps));
- goto prepare_wstring;
- }
- }
- else
- {
- prepare_wstring:
- n = mbsrtowcs (NULL, &string, 0, &ps);
- if (__glibc_unlikely (n == (size_t) -1))
- /* Something wrong.
- XXX Do we have to set 'errno' to something which mbsrtows hasn't
- already done? */
- goto free_return;
- if (__glibc_unlikely (n >= (size_t) -1 / sizeof (wchar_t)))
- {
- free (wpattern_malloc);
- __set_errno (ENOMEM);
- return -2;
- }
-
- wstring_malloc = wstring
- = (wchar_t *) malloc ((n + 1) * sizeof (wchar_t));
- if (wstring == NULL)
- {
- free (wpattern_malloc);
- return -2;
- }
- assert (mbsinit (&ps));
- (void) mbsrtowcs (wstring, &string, n + 1, &ps);
- }
-
- int res = internal_fnwmatch (wpattern, wstring, wstring + n,
+ r = fnmatch_convert_to_wide (string, &wstring, &n);
+ if (r == 0)
+ r = internal_fnwmatch (wpattern.data, wstring.data,
+ (wchar_t *) wstring.data + n,
flags & FNM_PERIOD, flags, NULL,
- alloca_used);
+ false);
+ }
- free (wstring_malloc);
- free (wpattern_malloc);
+ scratch_buffer_free (&wstring);
+ scratch_buffer_free (&wpattern);
- return res;
+ if (r == -2 || r == 0)
+ return r;
}
return internal_fnmatch (pattern, string, string + strlen (string),
diff --git a/posix/tst-fnmatch.input b/posix/tst-fnmatch.input
index 4fef4ee829..67aac5aada 100644
--- a/posix/tst-fnmatch.input
+++ b/posix/tst-fnmatch.input
@@ -676,6 +676,8 @@ C "x" "*" 0 PATHNAME|LEADING_DIR
C "x/y" "*" 0 PATHNAME|LEADING_DIR
C "x/y/z" "*" 0 PATHNAME|LEADING_DIR
C "x" "*x" 0 PATHNAME|LEADING_DIR
+
+en_US.UTF-8 "\366.csv" "*.csv" 0
C "x/y" "*x" 0 PATHNAME|LEADING_DIR
C "x/y/z" "*x" 0 PATHNAME|LEADING_DIR
C "x" "x*" 0 PATHNAME|LEADING_DIR
--
2.25.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 2/2] posix: Remove alloca usage for internal fnmatch implementation
2021-02-02 13:08 [PATCH 1/2] posix: Falling back to non wide mode in case of encoding error [BZ #14185] Adhemerval Zanella
@ 2021-02-02 13:08 ` Adhemerval Zanella
2021-02-22 10:54 ` [PATCH 1/2] posix: Falling back to non wide mode in case of encoding error [BZ #14185] Florian Weimer
1 sibling, 0 replies; 4+ messages in thread
From: Adhemerval Zanella @ 2021-02-02 13:08 UTC (permalink / raw)
To: libc-alpha; +Cc: bug-gnulib, Paul Eggert
This patch replaces the internal fnmatch pattern list generation
to use a dynamic array.
Checked on x86_64-linux-gnu.
---
posix/fnmatch.c | 24 +-----
posix/fnmatch_loop.c | 190 +++++++++++++++++++------------------------
2 files changed, 87 insertions(+), 127 deletions(-)
diff --git a/posix/fnmatch.c b/posix/fnmatch.c
index a66c9196c7..51080ab833 100644
--- a/posix/fnmatch.c
+++ b/posix/fnmatch.c
@@ -31,9 +31,6 @@
#include <ctype.h>
#include <string.h>
#include <stdlib.h>
-#if defined _LIBC || HAVE_ALLOCA
-# include <alloca.h>
-#endif
#include <wchar.h>
#include <wctype.h>
#include <stddef.h>
@@ -87,22 +84,6 @@ typedef ptrdiff_t idx_t;
#define NO_LEADING_PERIOD(flags) \
((flags & (FNM_FILE_NAME | FNM_PERIOD)) == (FNM_FILE_NAME | FNM_PERIOD))
-#ifndef _LIBC
-# if HAVE_ALLOCA
-/* The OS usually guarantees only one guard page at the bottom of the stack,
- and a page size can be as small as 4096 bytes. So we cannot safely
- allocate anything larger than 4096 bytes. Also care for the possibility
- of a few compiler-allocated temporary stack slots. */
-# define __libc_use_alloca(n) ((n) < 4032)
-# else
-/* Just use malloc. */
-# define __libc_use_alloca(n) false
-# undef alloca
-# define alloca(n) malloc (n)
-# endif
-# define alloca_account(size, avar) ((avar) += (size), alloca (size))
-#endif
-
/* Provide support for user-defined character classes, based on the functions
from ISO C 90 amendment 1. */
#ifdef CHARCLASS_NAME_MAX
@@ -289,8 +270,7 @@ fnmatch (const char *pattern, const char *string, int flags)
if (r == 0)
r = internal_fnwmatch (wpattern.data, wstring.data,
(wchar_t *) wstring.data + n,
- flags & FNM_PERIOD, flags, NULL,
- false);
+ flags & FNM_PERIOD, flags, NULL);
}
scratch_buffer_free (&wstring);
@@ -301,7 +281,7 @@ fnmatch (const char *pattern, const char *string, int flags)
}
return internal_fnmatch (pattern, string, string + strlen (string),
- flags & FNM_PERIOD, flags, NULL, 0);
+ flags & FNM_PERIOD, flags, NULL);
}
#undef fnmatch
diff --git a/posix/fnmatch_loop.c b/posix/fnmatch_loop.c
index 7f938af590..69f78f0fd8 100644
--- a/posix/fnmatch_loop.c
+++ b/posix/fnmatch_loop.c
@@ -30,15 +30,14 @@ struct STRUCT
it matches, nonzero if not. */
static int FCT (const CHAR *pattern, const CHAR *string,
const CHAR *string_end, bool no_leading_period, int flags,
- struct STRUCT *ends, size_t alloca_used);
+ struct STRUCT *ends);
static int 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 *string_end, bool no_leading_period, int flags);
static const CHAR *END (const CHAR *patternp);
static int
FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
- bool no_leading_period, int flags, struct STRUCT *ends, size_t alloca_used)
+ bool no_leading_period, int flags, struct STRUCT *ends)
{
const CHAR *p = pattern, *n = string;
UCHAR c;
@@ -62,8 +61,7 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
case L_('?'):
if (__glibc_unlikely (flags & FNM_EXTMATCH) && *p == '(')
{
- int res = EXT (c, p, n, string_end, no_leading_period,
- flags, alloca_used);
+ int res = EXT (c, p, n, string_end, no_leading_period, flags);
if (res != -1)
return res;
}
@@ -92,8 +90,7 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
case L_('*'):
if (__glibc_unlikely (flags & FNM_EXTMATCH) && *p == '(')
{
- int res = EXT (c, p, n, string_end, no_leading_period,
- flags, alloca_used);
+ int res = EXT (c, p, n, string_end, no_leading_period, flags);
if (res != -1)
return res;
}
@@ -182,7 +179,7 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
for (--p; n < endp; ++n, no_leading_period = false)
if (FCT (p, n, string_end, no_leading_period, flags2,
- &end, alloca_used) == 0)
+ &end) == 0)
goto found;
}
else if (c == L_('/') && (flags & FNM_FILE_NAME))
@@ -191,7 +188,7 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
++n;
if (n < string_end && *n == L_('/')
&& (FCT (p, n + 1, string_end, flags & FNM_PERIOD, flags,
- NULL, alloca_used) == 0))
+ NULL) == 0))
return 0;
}
else
@@ -205,7 +202,7 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
for (--p; n < endp; ++n, no_leading_period = false)
if (FOLD ((UCHAR) *n) == c
&& (FCT (p, n, string_end, no_leading_period, flags2,
- &end, alloca_used) == 0))
+ &end) == 0))
{
found:
if (end.pattern == NULL)
@@ -892,8 +889,7 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
case L_('!'):
if (__glibc_unlikely (flags & FNM_EXTMATCH) && *p == '(')
{
- int res = EXT (c, p, n, string_end, no_leading_period, flags,
- alloca_used);
+ int res = EXT (c, p, n, string_end, no_leading_period, flags);
if (res != -1)
return res;
}
@@ -972,26 +968,37 @@ END (const CHAR *pattern)
return p + 1;
}
+#if WIDE_CHAR_VERSION
+# define PATTERN_PREFIX pattern_list
+#else
+# define PATTERN_PREFIX wpattern_list
+#endif
+
+#define PASTE(a,b) PASTE1(a,b)
+#define PASTE1(a,b) a##b
+
+#define DYNARRAY_STRUCT PATTERN_PREFIX
+#define DYNARRAY_ELEMENT_FREE(ptr) free (*ptr)
+#define DYNARRAY_ELEMENT CHAR *
+#define DYNARRAY_PREFIX PASTE(PATTERN_PREFIX,_)
+#define DYNARRAY_INITIAL_SIZE 8
+#include <malloc/dynarray-skeleton.c>
static int
EXT (INT opt, const CHAR *pattern, const CHAR *string, const CHAR *string_end,
- bool no_leading_period, int flags, size_t alloca_used)
+ bool no_leading_period, int flags)
{
const CHAR *startp;
ptrdiff_t level;
- struct patternlist
- {
- struct patternlist *next;
- CHAR malloced;
- CHAR str __flexarr;
- } *list = NULL;
- struct patternlist **lastp = &list;
+ struct PATTERN_PREFIX list;
size_t pattern_len = STRLEN (pattern);
- bool any_malloced = false;
+ size_t pattern_i = 0;
const CHAR *p;
const CHAR *rs;
int retval = 0;
+ PASTE (PATTERN_PREFIX, _init) (&list);
+
/* Parse the pattern. Store the individual parts in the list. */
level = 0;
for (startp = p = pattern + 1; level >= 0; ++p)
@@ -1027,74 +1034,48 @@ EXT (INT opt, const CHAR *pattern, const CHAR *string, const CHAR *string_end,
|| *p == L_('!')) && p[1] == L_('('))
/* Remember the nesting level. */
++level;
- else if (*p == L_(')'))
- {
- if (level-- == 0)
- {
- /* This means we found the end of the pattern. */
-#define NEW_PATTERN \
- struct patternlist *newp; \
- size_t plen = (opt == L_('?') || opt == L_('@') \
- ? pattern_len : (p - startp + 1UL)); \
- idx_t slen = FLEXSIZEOF (struct patternlist, str, 0); \
- idx_t new_used = alloca_used + slen; \
- idx_t plensize; \
- if (INT_MULTIPLY_WRAPV (plen, sizeof (CHAR), &plensize) \
- || INT_ADD_WRAPV (new_used, plensize, &new_used)) \
- { \
- retval = -2; \
- goto out; \
- } \
- slen += plensize; \
- bool malloced = ! __libc_use_alloca (new_used); \
- if (__glibc_unlikely (malloced)) \
- { \
- newp = malloc (slen); \
- if (newp == NULL) \
- { \
- retval = -2; \
- goto out; \
- } \
- any_malloced = true; \
- } \
- else \
- newp = alloca_account (slen, alloca_used); \
- newp->next = NULL; \
- newp->malloced = malloced; \
- *((CHAR *) MEMPCPY (newp->str, startp, p - startp)) = L_('\0'); \
- *lastp = newp; \
- lastp = &newp->next
- NEW_PATTERN;
- }
- }
- else if (*p == L_('|'))
+ else if (*p == L_(')') || *p == L_('|'))
{
if (level == 0)
{
- NEW_PATTERN;
- startp = p + 1;
+ size_t slen = opt == L_('?') || opt == L_('@')
+ ? pattern_len : p - startp + 1;
+ CHAR *newp = malloc (slen * sizeof (CHAR));
+ if (newp != NULL)
+ {
+ *((CHAR *) MEMPCPY (newp, startp, p - startp)) = L_('\0');
+ PASTE (PATTERN_PREFIX,_add) (&list, newp);
+ }
+ if (newp == NULL || PASTE (PATTERN_PREFIX, _has_failed) (&list))
+ {
+ retval = -2;
+ goto out;
+ }
+
+ if (*p == L_('|'))
+ startp = p + 1;
}
+ if (*p == L_(')'))
+ level--;
}
- assert (list != NULL);
assert (p[-1] == L_(')'));
-#undef NEW_PATTERN
switch (opt)
{
case L_('*'):
- if (FCT (p, string, string_end, no_leading_period, flags, NULL,
- alloca_used) == 0)
+ if (FCT (p, string, string_end, no_leading_period, flags, NULL) == 0)
goto success;
FALLTHROUGH;
case L_('+'):
- do
+ for (; pattern_i < PASTE (PATTERN_PREFIX, _size)(&list); pattern_i++)
{
for (rs = string; rs <= string_end; ++rs)
/* First match the prefix with the current pattern with the
current pattern. */
- if (FCT (list->str, string, rs, no_leading_period,
+ if (FCT (*PASTE (PATTERN_PREFIX, _at) (&list, pattern_i), string,
+ rs, no_leading_period,
flags & FNM_FILE_NAME ? flags : flags & ~FNM_PERIOD,
- NULL, alloca_used) == 0
+ NULL) == 0
/* This was successful. Now match the rest with the rest
of the pattern. */
&& (FCT (p, rs, string_end,
@@ -1102,7 +1083,7 @@ EXT (INT opt, const CHAR *pattern, const CHAR *string, const CHAR *string_end,
? no_leading_period
: rs[-1] == '/' && NO_LEADING_PERIOD (flags),
flags & FNM_FILE_NAME
- ? flags : flags & ~FNM_PERIOD, NULL, alloca_used) == 0
+ ? flags : flags & ~FNM_PERIOD, NULL) == 0
/* This didn't work. Try the whole pattern. */
|| (rs != string
&& FCT (pattern - 1, rs, string_end,
@@ -1110,35 +1091,33 @@ EXT (INT opt, const CHAR *pattern, const CHAR *string, const CHAR *string_end,
? no_leading_period
: rs[-1] == '/' && NO_LEADING_PERIOD (flags),
flags & FNM_FILE_NAME
- ? flags : flags & ~FNM_PERIOD, NULL,
- alloca_used) == 0)))
+ ? flags : flags & ~FNM_PERIOD, NULL) == 0)))
/* It worked. Signal success. */
goto success;
}
- while ((list = list->next) != NULL);
/* None of the patterns lead to a match. */
retval = FNM_NOMATCH;
break;
case L_('?'):
- if (FCT (p, string, string_end, no_leading_period, flags, NULL,
- alloca_used) == 0)
+ if (FCT (p, string, string_end, no_leading_period, flags, NULL) == 0)
goto success;
FALLTHROUGH;
case L_('@'):
- do
- /* I cannot believe it but 'strcat' is actually acceptable
- here. Match the entire string with the prefix from the
- pattern list and the rest of the pattern following the
- pattern list. */
- if (FCT (STRCAT (list->str, p), string, string_end,
- no_leading_period,
- flags & FNM_FILE_NAME ? flags : flags & ~FNM_PERIOD,
- NULL, alloca_used) == 0)
- /* It worked. Signal success. */
- goto success;
- while ((list = list->next) != NULL);
+ for (; pattern_i < PASTE (PATTERN_PREFIX, _size) (&list); pattern_i++)
+ {
+ /* I cannot believe it but `strcat' is actually acceptable
+ here. Match the entire string with the prefix from the
+ pattern list and the rest of the pattern following the
+ pattern list. */
+ if (FCT (STRCAT (*PASTE (PATTERN_PREFIX, _at) (&list, pattern_i), p),
+ string, string_end, no_leading_period,
+ flags & FNM_FILE_NAME ? flags : flags & ~FNM_PERIOD,
+ NULL) == 0)
+ /* It worked. Signal success. */
+ goto success;
+ }
/* None of the patterns lead to a match. */
retval = FNM_NOMATCH;
@@ -1147,22 +1126,27 @@ EXT (INT opt, const CHAR *pattern, const CHAR *string, const CHAR *string_end,
case L_('!'):
for (rs = string; rs <= string_end; ++rs)
{
- struct patternlist *runp;
+ size_t runp_i;
- for (runp = list; runp != NULL; runp = runp->next)
- if (FCT (runp->str, string, rs, no_leading_period,
- flags & FNM_FILE_NAME ? flags : flags & ~FNM_PERIOD,
- NULL, alloca_used) == 0)
+ for (runp_i = pattern_i;
+ runp_i != PASTE (PATTERN_PREFIX, _size) (&list);
+ runp_i++)
+ {
+ if (FCT (*PASTE (PATTERN_PREFIX, _at) (&list, runp_i), string, rs,
+ no_leading_period,
+ flags & FNM_FILE_NAME ? flags : flags & ~FNM_PERIOD,
+ NULL) == 0)
break;
+ }
/* If none of the patterns matched see whether the rest does. */
- if (runp == NULL
+ if (runp_i == PASTE (PATTERN_PREFIX, _size) (&list)
&& (FCT (p, rs, string_end,
rs == string
? no_leading_period
: rs[-1] == '/' && NO_LEADING_PERIOD (flags),
flags & FNM_FILE_NAME ? flags : flags & ~FNM_PERIOD,
- NULL, alloca_used) == 0))
+ NULL) == 0))
/* This is successful. */
goto success;
}
@@ -1180,18 +1164,14 @@ EXT (INT opt, const CHAR *pattern, const CHAR *string, const CHAR *string_end,
success:
out:
- if (any_malloced)
- while (list != NULL)
- {
- struct patternlist *old = list;
- list = list->next;
- if (old->malloced)
- free (old);
- }
+ PASTE (PATTERN_PREFIX, _free) (&list);
return retval;
}
+#undef PATTERN_PREFIX
+#undef PASTE
+#undef PASTE1
#undef FOLD
#undef CHAR
--
2.25.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] posix: Falling back to non wide mode in case of encoding error [BZ #14185]
2021-02-02 13:08 [PATCH 1/2] posix: Falling back to non wide mode in case of encoding error [BZ #14185] Adhemerval Zanella
2021-02-02 13:08 ` [PATCH 2/2] posix: Remove alloca usage for internal fnmatch implementation Adhemerval Zanella
@ 2021-02-22 10:54 ` Florian Weimer
2021-02-22 10:56 ` Florian Weimer
1 sibling, 1 reply; 4+ messages in thread
From: Florian Weimer @ 2021-02-22 10:54 UTC (permalink / raw)
To: Adhemerval Zanella via Libc-alpha; +Cc: Adhemerval Zanella, bug-gnulib
* Adhemerval Zanella via Libc-alpha:
> +static int
> +fnmatch_convert_to_wide (const char *str, struct scratch_buffer *buf,
> + size_t *n)
> +{
> + mbstate_t ps;
> + memset (&ps, '\0', sizeof (ps));
> +
> + size_t nw = buf->length / sizeof (wchar_t);
> + *n = strnlen (str, nw - 1);
> + if (__glibc_likely (*n < nw))
> + {
> + const char *p = str;
> + *n = mbsrtowcs (buf->data, &p, *n + 1, &ps);
> + if (__glibc_unlikely (*n == (size_t) -1))
> + /* Something wrong.
> + XXX Do we have to set 'errno' to something which mbsrtows hasn't
> + already done? */
> + return -1;
> + if (p == NULL)
> + return 0;
> + memset (&ps, '\0', sizeof (ps));
> + }
> +
> + *n = mbsrtowcs (NULL, &str, 0, &ps);
> + if (__glibc_unlikely (*n == (size_t) -1))
> + return -1;
> + if (!scratch_buffer_set_array_size (buf, *n + 1, sizeof (wchar_t)))
> + {
> + __set_errno (ENOMEM);
> + return -2;
> + }
> + assert (mbsinit (&ps));
> + mbsrtowcs (buf->data, &str, *n + 1, &ps);
> + return 0;
> +}
>
This, along with
> + if (r == -2 || r == 0)
> + return r;
below causes fnmatch to return the undocumented -2 error value, I think.
Shouldn't we keep failing with -1 on error?
Thanks,
Florian
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] posix: Falling back to non wide mode in case of encoding error [BZ #14185]
2021-02-22 10:54 ` [PATCH 1/2] posix: Falling back to non wide mode in case of encoding error [BZ #14185] Florian Weimer
@ 2021-02-22 10:56 ` Florian Weimer
0 siblings, 0 replies; 4+ messages in thread
From: Florian Weimer @ 2021-02-22 10:56 UTC (permalink / raw)
To: Florian Weimer via Libc-alpha; +Cc: bug-gnulib
* Florian Weimer via Libc-alpha:
> * Adhemerval Zanella via Libc-alpha:
>
>> +static int
>> +fnmatch_convert_to_wide (const char *str, struct scratch_buffer *buf,
>> + size_t *n)
>> +{
>> + mbstate_t ps;
>> + memset (&ps, '\0', sizeof (ps));
>> +
>> + size_t nw = buf->length / sizeof (wchar_t);
>> + *n = strnlen (str, nw - 1);
>> + if (__glibc_likely (*n < nw))
>> + {
>> + const char *p = str;
>> + *n = mbsrtowcs (buf->data, &p, *n + 1, &ps);
>> + if (__glibc_unlikely (*n == (size_t) -1))
>> + /* Something wrong.
>> + XXX Do we have to set 'errno' to something which mbsrtows hasn't
>> + already done? */
>> + return -1;
>> + if (p == NULL)
>> + return 0;
>> + memset (&ps, '\0', sizeof (ps));
>> + }
>> +
>> + *n = mbsrtowcs (NULL, &str, 0, &ps);
>> + if (__glibc_unlikely (*n == (size_t) -1))
>> + return -1;
>> + if (!scratch_buffer_set_array_size (buf, *n + 1, sizeof (wchar_t)))
>> + {
>> + __set_errno (ENOMEM);
>> + return -2;
>> + }
>> + assert (mbsinit (&ps));
>> + mbsrtowcs (buf->data, &str, *n + 1, &ps);
>> + return 0;
>> +}
>>
>
> This, along with
>
>> + if (r == -2 || r == 0)
>> + return r;
>
> below causes fnmatch to return the undocumented -2 error value, I think.
> Shouldn't we keep failing with -1 on error?
I see that this is what the existing code does. So the patch looks okay
to me after all.
Thanks,
Florian
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-02-22 10:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-02 13:08 [PATCH 1/2] posix: Falling back to non wide mode in case of encoding error [BZ #14185] Adhemerval Zanella
2021-02-02 13:08 ` [PATCH 2/2] posix: Remove alloca usage for internal fnmatch implementation Adhemerval Zanella
2021-02-22 10:54 ` [PATCH 1/2] posix: Falling back to non wide mode in case of encoding error [BZ #14185] Florian Weimer
2021-02-22 10:56 ` Florian Weimer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).