diff mbox

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

Message ID 8d0ee88e-9c0d-1fa3-e010-cdc95efc2f64@cs.ucla.edu
State New, archived
Headers show

Commit Message

Paul Eggert Sept. 26, 2017, 6:10 p.m. UTC
On 09/26/2017 10:24 AM, Adhemerval Zanella wrote:
> I see other possible issues that would
> need to be fixed as well:
>
>    * FLEXIBLE_ARRAY_MEMBER definition for !__LIBC.
>    * __glob_pattern_type duplicated prototype.
>    * __set_errno and mempcpy definition.

Thanks for reporting the duplicated prototype; that is a portability 
issue that I fixed in Gnulib by applying the attached patch to Gnulib 
master. Gnulib glob.c already handles FLEXIBLE_ARRAY_MEMBER, 
__set_errno, and mempcpy automatically, because its 'glob' module has 
the modules 'flexmember', 'libc-config', and 'mempcpy' as prerequisites, 
and these supply the necessary definitions.

You can test Gnulib glob.c's portabililty by running this shell command 
in Gnulib's top directory:

./gnulib-tool --create-testdir --dir foo glob

and then by copying the newly-created 'foo' directory to the platform of 
your choice and running './configure; make check' there.

Gnu Make does not use Gnulib. Instead, it ships an old version of 
glob.c, which it took directly from glibc in 1999; this all predates 
Gnulib. If GNU Make ever updates to a more-modern glob.c I suggest that 
it use the Gnulib glob module, along with that module's prerequisites. I 
could help do that, if the GNU Make maintainer wants to go that route. 
[I'll CC: this to bug-make to give its maintainer a heads-up about this 
libc-alpha discussion, which can be followed from here:

https://sourceware.org/ml/libc-alpha/2017-09/msg00972.html

]

> I could build gnumake with forced internal glob implementation
> (make_cv_sys_gnu_glob=no) with the patch following patch.  Looking
> gnulib I am not sure if the correct way to use mempcpy for !_LIBC.

Sorry, but I'm confused by that patch (quoted below). It is a patch to 
glibc, so at first I assumed that you applied it to a copy of the glibc 
source, built a new glibc with it, and then used the newly-built glibc 
to configure and build GNU Make 4.2.1.  But that assumption cannot be 
right, since the patch modifies only code that is unused when building 
glibc. And one cannot simply copy current or patched glibc posix/glob.c 
into make's lib/glob.c, as (among other things) glibc's posix/glob.c 
includes <flexmember.h> when compiled outside of glibc, and GNU Make 
does not have that include file. So how did you use this patch?

> diff --git a/posix/glob.c b/posix/glob.c
> index 98122dac88..31e3aba4dd 100644
> --- a/posix/glob.c
> +++ b/posix/glob.c
> @@ -65,11 +65,15 @@
>   # define __stat64(fname, buf)   stat (fname, buf)
>   # define __fxstatat64(_, d, f, st, flag) fstatat (d, f, st, flag)
>   # define struct_stat64          struct stat
> -# ifndef __MVS__
> -#  define __alloca              alloca
> -# endif
> +# define __alloca               alloca
>   # define __readdir              readdir
> +# define FLEXIBLE_ARRAY_MEMBER 0
>   # define COMPILE_GLOB64
> +static inline void *
> +mempcpy (void *dest, const void *src, size_t n)
> +{
> +  return memcpy (dest, src, n) + n;
> +}
>   #endif /* _LIBC */
>   
>   #include <fnmatch.h>
> @@ -230,8 +234,6 @@ glob_use_alloca (size_t alloca_used, size_t len)
>   static int glob_in_dir (const char *pattern, const char *directory,
>                          int flags, int (*errfunc) (const char *, int),
>                          glob_t *pglob, size_t alloca_used);
> -extern int __glob_pattern_type (const char *pattern, int quote)
> -    attribute_hidden;
>   
>   static int prefix_array (const char *prefix, char **array, size_t n) __THROWNL;
>   static int collated_compare (const void *, const void *) __THROWNL;
>
>

Comments

Adhemerval Zanella Netto Sept. 26, 2017, 9:40 p.m. UTC | #1
On 26/09/2017 11:10, Paul Eggert wrote:
> On 09/26/2017 10:24 AM, Adhemerval Zanella wrote:
>> I see other possible issues that would
>> need to be fixed as well:
>>
>>    * FLEXIBLE_ARRAY_MEMBER definition for !__LIBC.
>>    * __glob_pattern_type duplicated prototype.
>>    * __set_errno and mempcpy definition.
>
> Thanks for reporting the duplicated prototype; that is a portability
> issue that I fixed in Gnulib by applying the attached patch to Gnulib
> master. Gnulib glob.c already handles FLEXIBLE_ARRAY_MEMBER,
> __set_errno, and mempcpy automatically, because its 'glob' module has
> the modules 'flexmember', 'libc-config', and 'mempcpy' as
> prerequisites, and these supply the necessary definitions.
Thanks, I will sync this change with glibc.

>
> You can test Gnulib glob.c's portabililty by running this shell
> command in Gnulib's top directory:
>
> ./gnulib-tool --create-testdir --dir foo glob
>
> and then by copying the newly-created 'foo' directory to the platform
> of your choice and running './configure; make check' there.
>
> Gnu Make does not use Gnulib. Instead, it ships an old version of
> glob.c, which it took directly from glibc in 1999; this all predates
> Gnulib. If GNU Make ever updates to a more-modern glob.c I suggest
> that it use the Gnulib glob module, along with that module's
> prerequisites. I could help do that, if the GNU Make maintainer wants
> to go that route. [I'll CC: this to bug-make to give its maintainer a
> heads-up about this libc-alpha discussion, which can be followed from
> here:
>
> https://sourceware.org/ml/libc-alpha/2017-09/msg00972.html

Besides update GNU make glob implementation I think it should also update
its configure.ac to accept not only _GNU_GLOB_INTERFACE_VERSION equal
to 1, but also _GNU_GLOB_INTERFACE_VERSION to 2 now that your suggestion [1]
has been fixed.

[1] http://lists.gnu.org/archive/html/bug-make/2017-09/msg00014.html

>
> ]
>
>> I could build gnumake with forced internal glob implementation
>> (make_cv_sys_gnu_glob=no) with the patch following patch.  Looking
>> gnulib I am not sure if the correct way to use mempcpy for !_LIBC.
>
> Sorry, but I'm confused by that patch (quoted below). It is a patch to
> glibc, so at first I assumed that you applied it to a copy of the
> glibc source, built a new glibc with it, and then used the newly-built
> glibc to configure and build GNU Make 4.2.1.  But that assumption
> cannot be right, since the patch modifies only code that is unused
> when building glibc. And one cannot simply copy current or patched
> glibc posix/glob.c into make's lib/glob.c, as (among other things)
> glibc's posix/glob.c includes <flexmember.h> when compiled outside of
> glibc, and GNU Make does not have that include file. So how did you
> use this patch?
Sorry if I were not explicit, the patch is indeed against glibc and what
I did was
basically copy glob.c and all required files (flexmember.h and
glob_internal)
on glob folder. It is just a ad hoc way to verify if its build correctly.

>
>> diff --git a/posix/glob.c b/posix/glob.c
>> index 98122dac88..31e3aba4dd 100644
>> --- a/posix/glob.c
>> +++ b/posix/glob.c
>> @@ -65,11 +65,15 @@
>>   # define __stat64(fname, buf)   stat (fname, buf)
>>   # define __fxstatat64(_, d, f, st, flag) fstatat (d, f, st, flag)
>>   # define struct_stat64          struct stat
>> -# ifndef __MVS__
>> -#  define __alloca              alloca
>> -# endif
>> +# define __alloca               alloca
>>   # define __readdir              readdir
>> +# define FLEXIBLE_ARRAY_MEMBER 0
>>   # define COMPILE_GLOB64
>> +static inline void *
>> +mempcpy (void *dest, const void *src, size_t n)
>> +{
>> +  return memcpy (dest, src, n) + n;
>> +}
>>   #endif /* _LIBC */
>>     #include <fnmatch.h>
>> @@ -230,8 +234,6 @@ glob_use_alloca (size_t alloca_used, size_t len)
>>   static int glob_in_dir (const char *pattern, const char *directory,
>>                          int flags, int (*errfunc) (const char *, int),
>>                          glob_t *pglob, size_t alloca_used);
>> -extern int __glob_pattern_type (const char *pattern, int quote)
>> -    attribute_hidden;
>>     static int prefix_array (const char *prefix, char **array, size_t
>> n) __THROWNL;
>>   static int collated_compare (const void *, const void *) __THROWNL;
>>
>>
diff mbox

Patch

From 3869eefa5b430ec8b23e71b5f150c6a7d8aacb50 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 26 Sep 2017 11:02:26 -0700
Subject: [PATCH] glob: remove bogus extern decl

* lib/glob.c (__glob_pattern_type): Remove now-spurious
extern declaration.  Problem reported by Adhemerval Zanella in:
https://sourceware.org/ml/libc-alpha/2017-09/msg00972.html
---
 ChangeLog  | 7 +++++++
 lib/glob.c | 3 ---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 4585cf12b..440b90686 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@ 
+2017-09-26  Paul Eggert  <eggert@cs.ucla.edu>
+
+	glob: remove bogus extern decl
+	* lib/glob.c (__glob_pattern_type): Remove now-spurious
+	extern declaration.  Problem reported by Adhemerval Zanella in:
+	https://sourceware.org/ml/libc-alpha/2017-09/msg00972.html
+
 2017-09-25  Paul Eggert  <eggert@cs.ucla.edu>
 
 	uniname/uniname-tests: integer overflow fix
diff --git a/lib/glob.c b/lib/glob.c
index 2a5e6642e..9d677d982 100644
--- a/lib/glob.c
+++ b/lib/glob.c
@@ -205,9 +205,6 @@  glob_use_alloca (size_t alloca_used, size_t len)
 static int glob_in_dir (const char *pattern, const char *directory,
                         int flags, int (*errfunc) (const char *, int),
                         glob_t *pglob, size_t alloca_used);
-extern int __glob_pattern_type (const char *pattern, int quote)
-    attribute_hidden;
-
 static int prefix_array (const char *prefix, char **array, size_t n) __THROWNL;
 static int collated_compare (const void *, const void *) __THROWNL;
 
-- 
2.13.5