Re: [PATCH] glob: Avoid copying the d_name field of struct dirent [BZ #19779]

[Roland McGrath <roland at hack dot frob dot com>]

>  > +# if defined _DIRENT_HAVE_D_TYPE || defined HAVE_STRUCT_DIRENT_D_TYPE
>  > +  __typeof__ (((struct dirent) {}).d_type) type;
>  > +# endif
> 
> __typeof__ isn't portable to non-GCC compilers. How about if we just 
> declare d_type to be 'unsigned char'? That should be portable enough in 
> practice.

Fine by me.

> Gnulib still ports to C89, which makes compound literals dicey. We've 
> been thinking of upping that to C99 so this is not an insurmountable 
> obstacle (so long as the code sticks to C99-compatible compound 
> literals, which I think it does). Still, this macro and its relatives 
> are a bit tricky, and if we're going to use tricky macros anyway perhaps 
> we'd be better off having them expand to C89 as that shouldn't make 
> things much more obscure.

In this case I think it's adequate to use a non-initializing
definition followed by assignments.  That avoids the vagaries of
relying on field order.  Florian can verify that it compiles to the
same thing, which I expect it will.

>  > +/* True if the directory entry D might be a symbolic link.  */
>  > +static inline bool
> 
> In Gnulib we typically prefer to say just 'static' without the clutter 
> of 'inline', for the same reason we typically don't bother with the 
> clutter of 'register'.

Actually, in libc it's our standard style to use just "static" in C
source files.  The principle is that the compiler will decide whether
to inline.  I failed to catch this style violation in my review.  (We
do still use "static inline" in header files, but basically just
because it's shorter to write than "static __attribute__ ((unused))".)

>  > +# define GL_READDIR(pglob, stream) (pglob)->gl_readdir ((stream))
> 
> No need for double parenthesization.

Canonical parenthesization would be:

# define GL_READDIR(pglob, stream) ((pglob)->gl_readdir (stream))


Thanks,
Roland