Discussion:
[PATCH 8/9] posix: Use enum for __glob_pattern_type result
Paul Eggert
2017-09-06 04:18:08 UTC
Permalink
+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.
Adhemerval Zanella
2017-09-06 13:03:53 UTC
Permalink
+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.
Paul Eggert
2017-09-06 16:18:55 UTC
Permalink
Post by Adhemerval Zanella
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.
Adhemerval Zanella
2017-09-06 16:54:23 UTC
Permalink
Post by Adhemerval Zanella
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.

Loading...