[PATCH] glob: Avoid copying the d_name field of struct dirent [BZ #19779]
Roland McGrath
roland@hack.frob.com
Fri Apr 1 21:08:00 GMT 2016
> > +# 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
More information about the Libc-alpha
mailing list