Re: [PATCH v2 1/2] posix: Add compat glob symbol to not follow dangling symbols

[Paul Eggert <eggert at cs dot ucla dot edu>]

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));