This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: PING^1: [PATCH 2/2] Mark internal unistd functions hidden in ld.so
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: libc-alpha at sourceware dot org
- Date: Wed, 6 Jan 2016 17:44:48 -0200
- Subject: Re: PING^1: [PATCH 2/2] Mark internal unistd functions hidden in ld.so
- Authentication-results: sourceware.org; auth=none
- References: <CAMe9rOqTP+-eodJn4W7vHogQQQb57q3cyrKXy1Wx5yTWQhCStQ at mail dot gmail dot com>
LGTM, only one comment below regarding some comments inclusion.
The only nit about the patchset is the creation of another header with
platform specific header. I think we can move both dl-mman.h and
dl-unistd.h to common header (maybe dl-sysdep.h), but we can push it
a future cleanup.
On 14-12-2015 12:10, H.J. Lu wrote:
> Ping.
>
> On Mon, Dec 7, 2015 at 11:00 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> Since internal unistd functions are only used internally in ld.so and
>> libc.so, they can be made hidden. __close, __getcwd, __getpid,
>> __libc_read and __libc_write can't be hidden in ld.so on Hurd since they
>> will be preempted by the ones in libc.so after bootstrap.
>>
>> [BZ #19122]
>> * include/unistd.h [IS_IN (rtld)]: Include <dl-unistd.h>.
>> * sysdeps/generic/dl-unistd.h: New file.
>> * sysdeps/mach/hurd/dl-unistd.h: Likewise.
>> ---
>> include/unistd.h | 6 +++++-
>> sysdeps/generic/dl-unistd.h | 30 ++++++++++++++++++++++++++++++
>> sysdeps/mach/hurd/dl-unistd.h | 25 +++++++++++++++++++++++++
>> 3 files changed, 60 insertions(+), 1 deletion(-)
>> create mode 100644 sysdeps/generic/dl-unistd.h
>> create mode 100644 sysdeps/mach/hurd/dl-unistd.h
>>
>> diff --git a/include/unistd.h b/include/unistd.h
>> index cb41637..5152f64 100644
>> --- a/include/unistd.h
>> +++ b/include/unistd.h
>> @@ -158,7 +158,7 @@ rtld_hidden_proto (__libc_enable_secure)
>>
>>
>> /* Various internal function. */
>> -extern void __libc_check_standard_fds (void);
>> +extern void __libc_check_standard_fds (void) attribute_hidden;
>>
>>
>> /* Internal name for fork function. */
>> @@ -176,6 +176,10 @@ extern int __have_dup3 attribute_hidden;
>> extern int __getlogin_r_loginuid (char *name, size_t namesize)
>> attribute_hidden;
>>
>> +# if IS_IN (rtld)
>> +# include <dl-unistd.h>
>> +# endif
>> +
>> __END_DECLS
>> # endif
>>
>> diff --git a/sysdeps/generic/dl-unistd.h b/sysdeps/generic/dl-unistd.h
>> new file mode 100644
>> index 0000000..98da672
>> --- /dev/null
>> +++ b/sysdeps/generic/dl-unistd.h
>> @@ -0,0 +1,30 @@
>> +/* Functions with hidden attribute internal to ld.so, which are declared
>> + in include/unistd.h. Generic version.
>> + Copyright (C) 2015 Free Software Foundation, Inc.
>> + This file is part of the GNU C Library.
>> +
>> + The GNU C Library is free software; you can redistribute it and/or
>> + modify it under the terms of the GNU Lesser General Public
>> + License as published by the Free Software Foundation; either
>> + version 2.1 of the License, or (at your option) any later version.
>> +
>> + The GNU C Library is distributed in the hope that it will be useful,
>> + but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + Lesser General Public License for more details.
>> +
>> + You should have received a copy of the GNU Lesser General Public
>> + License along with the GNU C Library; if not, see
>> + <http://www.gnu.org/licenses/>. */
>> +
>> +extern __typeof (__access) __access attribute_hidden;
>> +extern __typeof (__brk) __brk attribute_hidden;
>> +extern __typeof (__close) __close attribute_hidden;
>> +extern __typeof (__getcwd) __getcwd attribute_hidden;
>> +extern __typeof (__getpid) __getpid attribute_hidden;
>> +extern __typeof (__libc_read) __libc_read attribute_hidden;
>> +extern __typeof (__libc_write) __libc_write attribute_hidden;
>> +extern __typeof (__lseek) __lseek attribute_hidden;
>> +extern __typeof (__profil) __profil attribute_hidden;
>> +extern __typeof (__read) __read attribute_hidden;
>> +extern __typeof (__sbrk) __sbrk attribute_hidden;
>> diff --git a/sysdeps/mach/hurd/dl-unistd.h b/sysdeps/mach/hurd/dl-unistd.h
>> new file mode 100644
>> index 0000000..1777a21
>> --- /dev/null
>> +++ b/sysdeps/mach/hurd/dl-unistd.h
>> @@ -0,0 +1,25 @@
>> +/* Functions with hidden attribute internal to ld.so, which are declared
>> + in include/unistd.h. Hurd version.
>> + Copyright (C) 2015 Free Software Foundation, Inc.
>> + This file is part of the GNU C Library.
>> +
>> + The GNU C Library is free software; you can redistribute it and/or
>> + modify it under the terms of the GNU Lesser General Public
>> + License as published by the Free Software Foundation; either
>> + version 2.1 of the License, or (at your option) any later version.
>> +
>> + The GNU C Library is distributed in the hope that it will be useful,
>> + but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + Lesser General Public License for more details.
>> +
>> + You should have received a copy of the GNU Lesser General Public
>> + License along with the GNU C Library; if not, see
>> + <http://www.gnu.org/licenses/>. */
>> +
I think you should also add a comment, as you did for dl-mman.h, explaining
why we can't hide __close, __getcwd, __getpid, _libc_read and __libc_write
on Hurd.
>> +extern __typeof (__access) __access attribute_hidden;
>> +extern __typeof (__brk) __brk attribute_hidden;
>> +extern __typeof (__lseek) __lseek attribute_hidden;
>> +extern __typeof (__profil) __profil attribute_hidden;
>> +extern __typeof (__read) __read attribute_hidden;
>> +extern __typeof (__sbrk) __sbrk attribute_hidden;
>> --
>> 2.5.0
>>
>
>
>