This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] glob: Avoid copying the d_name field of struct dirent [BZ #19779]
- From: Roland McGrath <roland at hack dot frob dot com>
- To: Paul Eggert <eggert at cs dot ucla dot edu>
- Cc: Florian Weimer <fweimer at redhat dot com>, GNU C Library <libc-alpha at sourceware dot org>
- Date: Fri, 1 Apr 2016 14:08:55 -0700 (PDT)
- Subject: Re: [PATCH] glob: Avoid copying the d_name field of struct dirent [BZ #19779]
- Authentication-results: sourceware.org; auth=none
- References: <56E339A7 dot 7060704 at redhat dot com> <20160311222757 dot DB90C2C3C24 at topped-with-meat dot com> <56FBBA94 dot 1040605 at redhat dot com> <56FC6835 dot 7060406 at cs dot ucla dot edu>
> > +# 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