| Message ID | 20170817205809.GA7760@gmail.com |
|---|---|
| State | New, archived |
| Headers | show |
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
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.
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
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.
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)