Re: [bug-gnulib] New GNULIB glob module?

[Derek Price]

Paul Eggert wrote:

>Derek Price <address@hidden> writes:
>
>  
>
>> /* Enable GNU extensions in glob.h.  */
>>-#ifndef _GNU_SOURCE
>>+#if defined _LIBC && !defined _GNU_SOURCE
>> # define _GNU_SOURCE 1
>> #endif
>>    
>>
>
>I just checked the glibc source file include/libc-symbols.h, and it
>defines both _LIBC and _GNU_SOURCE.  So this stuff isn't needed at
>all; please remove these lines from glob.c instead.
>  
>

Done.

>>+   HAVE_STRUCT_DIRENT_D_TYPE is defined by GNULIB's glob.m4 when the same
>>+   member is found.  */
>>    
>>
>
>A bit wordy; please rephrase to "HAVE_STRUCT_DIRENT_D_TYPE plays the
>same rule in gnulib."
>  
>

Rephrased.

>>-      qsort ((__ptr_t) &pglob->gl_pathv[oldcount],
>>+      qsort ((void *) &pglob->gl_pathv[oldcount],
>>    
>>
>
>You can remove the cast entirely, right?
>  
>

So you agreed in an earlier message.  I've now removed all typecasts to
and from void * which are declared unnecessary by C89.

>>-       free ((__ptr_t) pglob->gl_pathv[pglob->gl_offs + i]);
>>-      free ((__ptr_t) pglob->gl_pathv);
>>+       free ((void *) pglob->gl_pathv[pglob->gl_offs + i]);
>>+      free ((void *) pglob->gl_pathv);
>>    
>>
>
>Similarly, these casts aren't needed.
>
>-          free ((__ptr_t) array[--i]);
>+          free ((void *) array[--i]);
>
>Nor these.
>
>  
>
>>-         free ((__ptr_t) array[--i]);
>>+         free ((void *) array[--i]);
>>    
>>
>
>Nor this one.
>
>  
>
>>+                       p = mempcpy ((void *) new->name, name, len);
>>    
>>
>
>Nor this one.
>
>  
>
>>-     free ((__ptr_t) names->name);
>>+     free ((void *) names->name);
>>    
>>
>
>Nor this.
>  
>

Removed, removed, removed, removed, and removed, per the above.

>>-  return (((flags & GLOB_ALTDIRFUNC)
>>-        ? (*pglob->gl_stat) (fullname, &st)
>>-        : __stat64 (fullname, &st64)) == 0);
>>+  return ((flags & GLOB_ALTDIRFUNC)
>>+       ? (*pglob->gl_stat) (fullname, &st) == 0 && S_ISDIR (st.st_mode)
>>+       : __stat64 (fullname, &st64) == 0 && S_ISDIR (st64.st_mode));
>>    
>>
>
>As long as you're reparenthesizing you might as well get rid of
>the unnecessary ones around (flags & GLOB_ALTDIRFUNC).
>  
>

Are you sure?  You asked me to restore similar parens around bit-ands
back at several other locations despite other work that changed the
lines, in an earlier email.  Not that I disagree now.  I actually prefer
the version without the unnecessary parens around the  bit-and.  I just
think we should be consistent.

>>+                       new->name = malloc (len + 1
>>+                                           + ((flags & GLOB_MARK) && isdir));
>>    
>>
>
>This line is 80 columns; should probably shrink it to 78, e.g., via.
>
>  new->name =
>    malloc (...);
>  
>

Done.

>glob.c is looking pretty good now.  I gotta run now, but I'll look at
>glob.h later today I hope.
>  
>

Thanks.  I'm not attaching a patch - I'll include a complete one
shortly, after I process your next two emails.

Cheers,

Derek