[PATCH v2 1/2] posix: Add compat glob symbol to not follow dangling symbols
Paul Eggert
eggert@cs.ucla.edu
Mon Sep 18 16:53:00 GMT 2017
I looked only at the posix/glob.c changes, since they're shared with Gnulib.
The macro GLOB_LSTAT_COMPAT has a confusing name. The name sounds like it
involves glob using lstat. But the name really means "glob should not call
lstat, to be compatible with older versions". I suggest renaming it to something
like GLOB_NO_LSTAT to make this clearer.
Why have a named type 'union glob_stat'? It's used only once, and the old code
didn't give it a name, so why give it a name now?
> + return __builtin_expect (flags & GLOB_ALTDIRFUNC, 0)
> +/* Use on glob-lstat-compat.c to provide a compat symbol which does not
> + use lstat / gl_lstat. */
> +#ifdef GLOB_LSTAT_COMPAT
> + ? (*pglob->gl_stat) (fullname, &ust.st)
> + : __stat64 (fullname, &ust.st64);
> +#else
> + ? (*pglob->gl_lstat) (fullname, &ust.st)
> + : __lstat64 (fullname, &ust.st64);
> +#endif
Please reformulate so that the #ifdef does not appear within a statement or
expression; it's confusing to have #ifs containing just parts of statements.
A minor point: there is no need for the '*' prefix operator; the use of '*' in
this code goes back to pre-C89 compilers and we don't need to worry about that
any more.
Perhaps something like the following instead? It addresses the above points.
#ifdef GLOB_NO_STAT
# define GL_LSTAT gl_stat
# define LSTAT64 __stat64
#else
# define GL_LSTAT gl_lstat
# define LSTAT64 __lstat64
#endif
...
union
{
struct stat st;
struct_stat64 st64;
} ust;
return (__glibc_unlikely (flags & GLOB_ALTDIRFUNC)
? pglob->GL_LSTAT (fullname, &ust.st)
: LSTAT64 (fullname, &ust.st64));
More information about the Libc-alpha
mailing list