[2/4] Mark __internal_statvfs[64] with attribute_hidden [BZ #18822]

Message ID 20170817205809.GA7760@gmail.com
State New, archived
Headers

Commit Message

Lu, Hongjiu Aug. 17, 2017, 8:58 p.m. UTC
  Mark __internal_statvfs[64] with attribute_hidden to allow direct access
to them within libc.so and libc.a without using GOT nor PLT.

Tested on i686 and x86-64.  OK for master?

H.J.
--
	[BZ #18822]
	* include/sys/statvfs.h (__internal_statvfs): New prototype.
	(__internal_statvfs64): Likewise.
	* sysdeps/unix/sysv/linux/fstatvfs.c (__internal_statvfs):
	Removed.
	* sysdeps/unix/sysv/linux/fstatvfs64.c (__internal_statvfs64):
	Likewise.
	* sysdeps/unix/sysv/linux/statvfs.c (__internal_statvfs):
	Likewise.
	* sysdeps/unix/sysv/linux/statvfs64.c (__internal_statvfs64):
	Likewise.
---
 include/sys/statvfs.h                | 8 ++++++++
 sysdeps/unix/sysv/linux/fstatvfs.c   | 4 ----
 sysdeps/unix/sysv/linux/fstatvfs64.c | 5 -----
 sysdeps/unix/sysv/linux/statvfs.c    | 4 ----
 sysdeps/unix/sysv/linux/statvfs64.c  | 5 -----
 5 files changed, 8 insertions(+), 18 deletions(-)
  

Comments

Florian Weimer Aug. 18, 2017, 12:55 p.m. UTC | #1
On 08/17/2017 10:58 PM, H.J. Lu wrote:
> Mark __internal_statvfs[64] with attribute_hidden to allow direct access
> to them within libc.so and libc.a without using GOT nor PLT.
> 
> Tested on i686 and x86-64.  OK for master?

This is a bit odd because the include/ headers are supposed to be
generic, and the declarations you add are Linux-specific.  In the past,
we would have added a Linux-specific header under
sysdeps/unix/sysv/linux for that.

I don't have a strong opinion either way.

Thanks,
Florian
  
H.J. Lu Aug. 18, 2017, 1:15 p.m. UTC | #2
On Fri, Aug 18, 2017 at 5:55 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 08/17/2017 10:58 PM, H.J. Lu wrote:
>> Mark __internal_statvfs[64] with attribute_hidden to allow direct access
>> to them within libc.so and libc.a without using GOT nor PLT.
>>
>> Tested on i686 and x86-64.  OK for master?
>
> This is a bit odd because the include/ headers are supposed to be
> generic, and the declarations you add are Linux-specific.  In the past,
> we would have added a Linux-specific header under
> sysdeps/unix/sysv/linux for that.
>
> I don't have a strong opinion either way.
>

Header files under include override both generic and non-generic header
files.  __KERNEL_STRICT_NAMES is another example.
  
Florian Weimer Aug. 18, 2017, 1:19 p.m. UTC | #3
On 08/18/2017 03:15 PM, H.J. Lu wrote:
> On Fri, Aug 18, 2017 at 5:55 AM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 08/17/2017 10:58 PM, H.J. Lu wrote:
>>> Mark __internal_statvfs[64] with attribute_hidden to allow direct access
>>> to them within libc.so and libc.a without using GOT nor PLT.
>>>
>>> Tested on i686 and x86-64.  OK for master?
>>
>> This is a bit odd because the include/ headers are supposed to be
>> generic, and the declarations you add are Linux-specific.  In the past,
>> we would have added a Linux-specific header under
>> sysdeps/unix/sysv/linux for that.
>>
>> I don't have a strong opinion either way.
>>
> 
> Header files under include override both generic and non-generic header
> files.  __KERNEL_STRICT_NAMES is another example.

Right.  I'm sure your patch works as intended (at least on most
architectures).  It's just a matter of policy to what extent we want to
uphold the fiction that upstream glibc supports more than just Linux.

Thanks,
Florian
  
Adhemerval Zanella Aug. 18, 2017, 1:24 p.m. UTC | #4
On 18/08/2017 10:19, Florian Weimer wrote:
> On 08/18/2017 03:15 PM, H.J. Lu wrote:
>> On Fri, Aug 18, 2017 at 5:55 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>> On 08/17/2017 10:58 PM, H.J. Lu wrote:
>>>> Mark __internal_statvfs[64] with attribute_hidden to allow direct access
>>>> to them within libc.so and libc.a without using GOT nor PLT.
>>>>
>>>> Tested on i686 and x86-64.  OK for master?
>>>
>>> This is a bit odd because the include/ headers are supposed to be
>>> generic, and the declarations you add are Linux-specific.  In the past,
>>> we would have added a Linux-specific header under
>>> sysdeps/unix/sysv/linux for that.
>>>
>>> I don't have a strong opinion either way.
>>>
>>
>> Header files under include override both generic and non-generic header
>> files.  __KERNEL_STRICT_NAMES is another example.
> 
> Right.  I'm sure your patch works as intended (at least on most
> architectures).  It's just a matter of policy to what extent we want to
> uphold the fiction that upstream glibc supports more than just Linux.

I also think it should be on a Linux only header, mostly for organization.
  

Patch

diff --git a/include/sys/statvfs.h b/include/sys/statvfs.h
index fa3045386d..1a36ff6450 100644
--- a/include/sys/statvfs.h
+++ b/include/sys/statvfs.h
@@ -6,6 +6,14 @@ 
 extern int __statvfs64 (const char *__file, struct statvfs64 *__buf);
 extern int __fstatvfs64 (int __fildes, struct statvfs64 *__buf);
 
+extern void __internal_statvfs (const char *name, struct statvfs *buf,
+				struct statfs *fsbuf, int fd)
+      attribute_hidden;
+extern void __internal_statvfs64 (const char *name, struct statvfs64 *buf,
+				  struct statfs64 *fsbuf, int fd)
+      attribute_hidden;
+
+
 libc_hidden_proto (statvfs)
 libc_hidden_proto (fstatvfs)
 #endif
diff --git a/sysdeps/unix/sysv/linux/fstatvfs.c b/sysdeps/unix/sysv/linux/fstatvfs.c
index e07a5b9f3d..0a947c30db 100644
--- a/sysdeps/unix/sysv/linux/fstatvfs.c
+++ b/sysdeps/unix/sysv/linux/fstatvfs.c
@@ -21,10 +21,6 @@ 
 #include <sys/statfs.h>
 #include <sys/statvfs.h>
 
-extern void __internal_statvfs (const char *name, struct statvfs *buf,
-				struct statfs *fsbuf, int fd);
-
-
 int
 __fstatvfs (int fd, struct statvfs *buf)
 {
diff --git a/sysdeps/unix/sysv/linux/fstatvfs64.c b/sysdeps/unix/sysv/linux/fstatvfs64.c
index 02a0d7ee42..bab3653e1b 100644
--- a/sysdeps/unix/sysv/linux/fstatvfs64.c
+++ b/sysdeps/unix/sysv/linux/fstatvfs64.c
@@ -23,11 +23,6 @@ 
 #include <sys/statvfs.h>
 #include <kernel-features.h>
 
-
-extern void __internal_statvfs64 (const char *name, struct statvfs64 *buf,
-				  struct statfs64 *fsbuf, int fd);
-
-
 /* Return information about the filesystem on which FD resides.  */
 int
 __fstatvfs64 (int fd, struct statvfs64 *buf)
diff --git a/sysdeps/unix/sysv/linux/statvfs.c b/sysdeps/unix/sysv/linux/statvfs.c
index 744336e33b..92aeb164ce 100644
--- a/sysdeps/unix/sysv/linux/statvfs.c
+++ b/sysdeps/unix/sysv/linux/statvfs.c
@@ -21,10 +21,6 @@ 
 #include <sys/statfs.h>
 #include <sys/statvfs.h>
 
-extern void __internal_statvfs (const char *name, struct statvfs *buf,
-				struct statfs *fsbuf, int fd);
-
-
 int
 __statvfs (const char *file, struct statvfs *buf)
 {
diff --git a/sysdeps/unix/sysv/linux/statvfs64.c b/sysdeps/unix/sysv/linux/statvfs64.c
index a89f720b38..ff4bdc61d1 100644
--- a/sysdeps/unix/sysv/linux/statvfs64.c
+++ b/sysdeps/unix/sysv/linux/statvfs64.c
@@ -24,11 +24,6 @@ 
 #include <sys/statvfs.h>
 #include <kernel-features.h>
 
-
-extern void __internal_statvfs64 (const char *name, struct statvfs64 *buf,
-				  struct statfs64 *fsbuf, int fd);
-
-
 /* Return information about the filesystem on which FILE resides.  */
 int
 __statvfs64 (const char *file, struct statvfs64 *buf)