[8/9] posix: Use enum for __glob_pattern_type result
Commit Message
Adhemerval Zanella wrote:
> +enum glob_pattern_type_t
> +{
> + __GLOB_NONE = 0x0,
> + __GLOB_SPECIAL = 0x1,
> + __GLOB_BACKSLASH = 0x2,
> + __GLOB_BRACKET = 0x4
> +};
The identifier glob_pattern_type_t is not used elsewhere, so let's omit it. This
makes it clearer that we're merely defining handy names for int constants, as
opposed to defining a new type.
Also, names like __GLOB_NONE could cause problems when Gnulib is used on non-GNU
platforms, which might use those names for other purposes. As glob_internal.h is
not user-visible, let's use ordinary names. I suggest GLOBPAT_NONE,
GLOBPAT_SPECIAL, etc., as done in the attached patch, which I installed into Gnulib.
Comments
On 06/09/2017 01:18, Paul Eggert wrote:
> Adhemerval Zanella wrote:
>> +enum glob_pattern_type_t
>> +{
>> + __GLOB_NONE = 0x0,
>> + __GLOB_SPECIAL = 0x1,
>> + __GLOB_BACKSLASH = 0x2,
>> + __GLOB_BRACKET = 0x4
>> +};
>
> The identifier glob_pattern_type_t is not used elsewhere, so let's omit it. This makes it clearer that we're merely defining handy names for int constants, as opposed to defining a new type.
Ack.
>
> Also, names like __GLOB_NONE could cause problems when Gnulib is used on non-GNU platforms, which might use those names for other purposes. As glob_internal.h is not user-visible, let's use ordinary names. I suggest GLOBPAT_NONE, GLOBPAT_SPECIAL, etc., as done in the attached patch, which I installed into Gnulib.
My understanding was double underscore identifiers are reserved for implementation
(C99 7.1.3 Reserved identifiers). But I do not have a strong opinion here, I am
ok with your approach.
Adhemerval Zanella wrote:
> My understanding was double underscore identifiers are reserved for implementation
> (C99 7.1.3 Reserved identifiers).
Yes, and that's the point. When this code is used as part of Gnulib, it is used
within an application, so any identifiers it uses that start with __ might
collide with the implementation, which means it's safer to avoid them when it's
easy, as is the case here.
On 06/09/2017 13:18, Paul Eggert wrote:
> Adhemerval Zanella wrote:
>> My understanding was double underscore identifiers are reserved for implementation
>> (C99 7.1.3 Reserved identifiers).
>
> Yes, and that's the point. When this code is used as part of Gnulib, it is used within an application, so any identifiers it uses that start with __ might collide with the implementation, which means it's safer to avoid them when it's easy, as is the case here.
Right, I got your point.
From 36102f8d365655b5d9693ccff0349acc73c60433 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 5 Sep 2017 21:14:51 -0700
Subject: [PATCH] glob: Use enum for __glob_pattern_type result
From a patch proposed by Adhemerval Zanella in:
https://sourceware.org/ml/libc-alpha/2017-09/msg00212.html
* lib/glob_internal.h (GLOBPAT_NONE, GLOBPAT_SPECIAL)
(GLOBPAT_BACKSLASH, GLOBPAT_BRACKET): New constants.
* lib/glob_internal.h (__glob_pattern_type):
* lib/glob.c (glob):
* lib/glob_pattern_p.c (__glob_pattern_p):
Use them.
---
ChangeLog | 10 ++++++++++
lib/glob.c | 8 ++++----
lib/glob_internal.h | 18 +++++++++++++-----
lib/glob_pattern_p.c | 2 +-
4 files changed, 28 insertions(+), 10 deletions(-)
@@ -1,5 +1,15 @@
2017-09-05 Paul Eggert <eggert@cs.ucla.edu>
+ glob: Use enum for __glob_pattern_type result
+ From a patch proposed by Adhemerval Zanella in:
+ https://sourceware.org/ml/libc-alpha/2017-09/msg00212.html
+ * lib/glob_internal.h (GLOBPAT_NONE, GLOBPAT_SPECIAL)
+ (GLOBPAT_BACKSLASH, GLOBPAT_BRACKET): New constants.
+ * lib/glob_internal.h (__glob_pattern_type):
+ * lib/glob.c (glob):
+ * lib/glob_pattern_p.c (__glob_pattern_p):
+ Use them.
+
glob: fix for use in glibc
Problem reported by Adhemerval Zanella in:
https://sourceware.org/ml/libc-alpha/2017-09/msg00213.html
@@ -903,7 +903,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
[ which we handle the same, using fnmatch. Broken unterminated
pattern bracket expressions ought to be rare enough that it is
not worth special casing them, fnmatch will do the right thing. */
- if (meta & 5)
+ if (meta & (GLOBPAT_SPECIAL | GLOBPAT_BRACKET))
{
/* The directory name contains metacharacters, so we
have to glob for the directory, and then glob for
@@ -1044,7 +1044,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
size_t old_pathc = pglob->gl_pathc;
int orig_flags = flags;
- if (meta & 2)
+ if (meta & GLOBPAT_BACKSLASH)
{
char *p = strchr (dirname, '\\'), *q;
/* We need to unescape the dirname string. It is certainly
@@ -1242,14 +1242,14 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
/ sizeof init_names->name[0]);
meta = __glob_pattern_type (pattern, !(flags & GLOB_NOESCAPE));
- if (meta == 0 && (flags & (GLOB_NOCHECK|GLOB_NOMAGIC)))
+ if (meta == GLOBPAT_NONE && (flags & (GLOB_NOCHECK|GLOB_NOMAGIC)))
{
/* We need not do any tests. The PATTERN contains no meta
characters and we must not return an error therefore the
result will always contain exactly one name. */
flags |= GLOB_NOCHECK;
}
- else if (meta == 0)
+ else if (meta == GLOBPAT_NONE)
{
union
{
@@ -19,35 +19,43 @@
#ifndef GLOB_INTERNAL_H
# define GLOB_INTERNAL_H
+enum
+{
+ GLOBPAT_NONE = 0x0,
+ GLOBPAT_SPECIAL = 0x1,
+ GLOBPAT_BACKSLASH = 0x2,
+ GLOBPAT_BRACKET = 0x4
+};
+
static inline int
__glob_pattern_type (const char *pattern, int quote)
{
const char *p;
- int ret = 0;
+ int ret = GLOBPAT_NONE;
for (p = pattern; *p != '\0'; ++p)
switch (*p)
{
case '?':
case '*':
- return 1;
+ return GLOBPAT_SPECIAL;
case '\\':
if (quote)
{
if (p[1] != '\0')
++p;
- ret |= 2;
+ ret |= GLOBPAT_BACKSLASH;
}
break;
case '[':
- ret |= 4;
+ ret |= GLOBPAT_BRACKET;
break;
case ']':
if (ret & 4)
- return 1;
+ return GLOBPAT_SPECIAL;
break;
}
@@ -28,6 +28,6 @@
int
__glob_pattern_p (const char *pattern, int quote)
{
- return __glob_pattern_type (pattern, quote) == 1;
+ return __glob_pattern_type (pattern, quote) == GLOBPAT_SPECIAL;
}
weak_alias (__glob_pattern_p, glob_pattern_p)
--
2.7.4