Message ID | 874l51hae1.fsf@oldenburg2.str.redhat.com |
---|---|
State | New |
Headers | show |
Series | Linux: Deprecate <sys/sysctl.h> and sysctl | expand |
On 07/06/2019 09:01, Florian Weimer wrote: > Now that there are no internal users of __sysctl left, it is possible > to add an unconditional deprecation warning to <sys/sysctl.h>. > > To avoid a test failure due this warning in check-install-headers, > skip the test for sys/sysctl.h. LGTM. As a side note, if I recall correctly one usage of sysctl that might be useful and not really available depending of the kernel version is to get some random bits from /proc/sys/kernel/random/uuid without opening /proc. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > 2019-06-07 Florian Weimer <fweimer@redhat.com> > > Linux: Deprecate sysctl. > * include/sysctl.h (__sysctl): Remove declaration. > * scripts/check-installed-headers.sh (sys/sysctl.h): Disable > check. > * sysdeps/unix/sysv/linux/sys/sysctl.h: Add deprecation warning. > (sysctl): Add deprecation attribute. > * sysdeps/unix/sysv/linux/sysctl.c: Include <linux/sysctl.h> > directly, to avoid the deprecation warning. Do not include > <string.h>. > (__sysctl): Remove hidden alias. Ok. > > diff --git a/NEWS b/NEWS > index 00f9e855a2..fffff79576 100644 > --- a/NEWS > +++ b/NEWS > @@ -61,6 +61,9 @@ Deprecated and removed features, and other changes affecting compatibility: > * On 32-bit Arm, support for the port-based I/O emulation and the <sys/io.h> > header have been removed. > > +* The Linux-specific <sys/sysctl.h> header and the sysctl function have been > + deprecated and will be removed from a future version of glibc. > + > Changes to build and runtime requirements: > > * GCC 6.2 or later is required to build the GNU C Library. Ok. > diff --git a/include/sys/sysctl.h b/include/sys/sysctl.h > index 2a15e91354..fa102aa226 100644 > --- a/include/sys/sysctl.h > +++ b/include/sys/sysctl.h > @@ -1,13 +1,3 @@ > #ifndef _SYS_SYSCTL_H > #include_next <sys/sysctl.h> > - > -# ifndef _ISOMAC > - > -/* Read or write system parameters (Linux, FreeBSD specific). */ > -extern int __sysctl (int *__name, int __nlen, void *__oldval, > - size_t *__oldlenp, void *__newval, size_t __newlen); > -libc_hidden_proto (__sysctl) > - > - > -# endif /* !_ISOMAC */ > #endif /* _SYS_SYSCTL_H */ Do we still need to internal file? Can't we just remove it? > diff --git a/scripts/check-installed-headers.sh b/scripts/check-installed-headers.sh > index e4f37d33f8..ef6ed94a2e 100644 > --- a/scripts/check-installed-headers.sh > +++ b/scripts/check-installed-headers.sh > @@ -53,7 +53,6 @@ trap "rm -f '$cih_test_c'" 0 > > failed=0 > is_x86_64=unknown > -is_x32=unknown > for header in "$@"; do > # Skip various headers for which this test gets a false failure. > case "$header" in > @@ -75,27 +74,10 @@ for header in "$@"; do > (finclude/*) > continue;; > > - # sys/sysctl.h is unsupported for x32. > + # sys/sysctl.h produces a deprecation warning and therefore > + # fails compilation with -Werror. > (sys/sysctl.h) > - case "$is_x32" in > - (yes) continue;; > - (no) ;; > - (unknown) > - cat >"$cih_test_c" <<EOF > -#if defined __x86_64__ && defined __ILP32__ > -# error "is x32" > -#endif > -EOF > - if $cc_cmd -fsyntax-only "$cih_test_c" > /dev/null 2>&1 > - then > - is_x32=no > - else > - is_x32=yes > - continue > - fi > - ;; > - esac > - ;; > + continue;; > > # sys/vm86.h is "unsupported on x86-64" and errors out on that target. > (sys/vm86.h) > diff --git a/sysdeps/unix/sysv/linux/sys/sysctl.h b/sysdeps/unix/sysv/linux/sys/sysctl.h > index 0f6e71ba7e..15ee9642ab 100644 > --- a/sysdeps/unix/sysv/linux/sys/sysctl.h > +++ b/sysdeps/unix/sysv/linux/sys/sysctl.h > @@ -18,6 +18,9 @@ > #ifndef _SYS_SYSCTL_H > #define _SYS_SYSCTL_H 1 > > +#warning "The <sys/sysctl.h> header is deprecated and will be removed." > +#warning "Directly access the /proc file system instead." > + > #include <features.h> > #define __need_size_t > #include <stddef.h> > @@ -66,7 +69,8 @@ __BEGIN_DECLS > > /* Read or write system parameters. */ > extern int sysctl (int *__name, int __nlen, void *__oldval, > - size_t *__oldlenp, void *__newval, size_t __newlen) __THROW; > + size_t *__oldlenp, void *__newval, size_t __newlen) __THROW > + __attribute_deprecated__; > > __END_DECLS > Ok. > diff --git a/sysdeps/unix/sysv/linux/sysctl.c b/sysdeps/unix/sysv/linux/sysctl.c > index 33afdd918b..0f18c69abe 100644 > --- a/sysdeps/unix/sysv/linux/sysctl.c > +++ b/sysdeps/unix/sysv/linux/sysctl.c > @@ -17,8 +17,7 @@ > <http://www.gnu.org/licenses/>. */ > > #include <errno.h> > -#include <string.h> /* For the real memset prototype. */ > -#include <sys/sysctl.h> > +#include <linux/sysctl.h> > > #include <sysdep.h> > #include <sys/syscall.h> > @@ -39,5 +38,4 @@ __sysctl (int *name, int nlen, void *oldval, size_t *oldlenp, > > return INLINE_SYSCALL (_sysctl, 1, &args); > } > -libc_hidden_def (__sysctl) > weak_alias (__sysctl, sysctl) > What about a link warning stating this interface is deprecated and it would be removed? My idea is to warn when people are static linking as well.
* Adhemerval Zanella: > On 07/06/2019 09:01, Florian Weimer wrote: >> Now that there are no internal users of __sysctl left, it is possible >> to add an unconditional deprecation warning to <sys/sysctl.h>. >> >> To avoid a test failure due this warning in check-install-headers, >> skip the test for sys/sysctl.h. > > LGTM. As a side note, if I recall correctly one usage of sysctl that might > be useful and not really available depending of the kernel version is to > get some random bits from /proc/sys/kernel/random/uuid without opening > /proc. Right, I saw this quite a lot in existing code (although some people use the FreeBSD MIB on Linux, which will always fail and never provide a UUID). >> +#warning "The <sys/sysctl.h> header is deprecated and will be removed." >> +#warning "Directly access the /proc file system instead." Should I add this as well? #warning "On current kernels, getentropy provides random bits without /proc." >> diff --git a/sysdeps/unix/sysv/linux/sysctl.c b/sysdeps/unix/sysv/linux/sysctl.c >> index 33afdd918b..0f18c69abe 100644 >> --- a/sysdeps/unix/sysv/linux/sysctl.c >> +++ b/sysdeps/unix/sysv/linux/sysctl.c >> @@ -17,8 +17,7 @@ >> <http://www.gnu.org/licenses/>. */ >> >> #include <errno.h> >> -#include <string.h> /* For the real memset prototype. */ >> -#include <sys/sysctl.h> >> +#include <linux/sysctl.h> >> >> #include <sysdep.h> >> #include <sys/syscall.h> >> @@ -39,5 +38,4 @@ __sysctl (int *name, int nlen, void *oldval, size_t *oldlenp, >> >> return INLINE_SYSCALL (_sysctl, 1, &args); >> } >> -libc_hidden_def (__sysctl) >> weak_alias (__sysctl, sysctl) >> > > What about a link warning stating this interface is deprecated and it would be > removed? My idea is to warn when people are static linking as well. They already get two warnings (from the header and the call site). Do we need a third warning? It's only helpful if you don't use the header, I think, but even with Go's cgo, you'll see the GCC warnings. Thanks, Florian
On 07/06/2019 13:01, Florian Weimer wrote: > * Adhemerval Zanella: > >> On 07/06/2019 09:01, Florian Weimer wrote: >>> Now that there are no internal users of __sysctl left, it is possible >>> to add an unconditional deprecation warning to <sys/sysctl.h>. >>> >>> To avoid a test failure due this warning in check-install-headers, >>> skip the test for sys/sysctl.h. >> >> LGTM. As a side note, if I recall correctly one usage of sysctl that might >> be useful and not really available depending of the kernel version is to >> get some random bits from /proc/sys/kernel/random/uuid without opening >> /proc. > > Right, I saw this quite a lot in existing code (although some people use > the FreeBSD MIB on Linux, which will always fail and never provide a UUID). > >>> +#warning "The <sys/sysctl.h> header is deprecated and will be removed." >>> +#warning "Directly access the /proc file system instead." > > Should I add this as well? > > #warning "On current kernels, getentropy provides random bits without /proc." Not sure, I think a note on NEWS might be better fort this specific usage. > >>> diff --git a/sysdeps/unix/sysv/linux/sysctl.c b/sysdeps/unix/sysv/linux/sysctl.c >>> index 33afdd918b..0f18c69abe 100644 >>> --- a/sysdeps/unix/sysv/linux/sysctl.c >>> +++ b/sysdeps/unix/sysv/linux/sysctl.c >>> @@ -17,8 +17,7 @@ >>> <http://www.gnu.org/licenses/>. */ >>> >>> #include <errno.h> >>> -#include <string.h> /* For the real memset prototype. */ >>> -#include <sys/sysctl.h> >>> +#include <linux/sysctl.h> >>> >>> #include <sysdep.h> >>> #include <sys/syscall.h> >>> @@ -39,5 +38,4 @@ __sysctl (int *name, int nlen, void *oldval, size_t *oldlenp, >>> >>> return INLINE_SYSCALL (_sysctl, 1, &args); >>> } >>> -libc_hidden_def (__sysctl) >>> weak_alias (__sysctl, sysctl) >>> >> >> What about a link warning stating this interface is deprecated and it would be >> removed? My idea is to warn when people are static linking as well. > > They already get two warnings (from the header and the call site). Do > we need a third warning? It's only helpful if you don't use the header, > I think, but even with Go's cgo, you'll see the GCC warnings. Fair enough. Patch LGTM, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
* Adhemerval Zanella: > On 07/06/2019 13:01, Florian Weimer wrote: >> * Adhemerval Zanella: >> >>> On 07/06/2019 09:01, Florian Weimer wrote: >>>> Now that there are no internal users of __sysctl left, it is possible >>>> to add an unconditional deprecation warning to <sys/sysctl.h>. >>>> >>>> To avoid a test failure due this warning in check-install-headers, >>>> skip the test for sys/sysctl.h. >>> >>> LGTM. As a side note, if I recall correctly one usage of sysctl that might >>> be useful and not really available depending of the kernel version is to >>> get some random bits from /proc/sys/kernel/random/uuid without opening >>> /proc. >> >> Right, I saw this quite a lot in existing code (although some people use >> the FreeBSD MIB on Linux, which will always fail and never provide a UUID). >> >>>> +#warning "The <sys/sysctl.h> header is deprecated and will be removed." >>>> +#warning "Directly access the /proc file system instead." >> >> Should I add this as well? >> >> #warning "On current kernels, getentropy provides random bits without /proc." > > Not sure, I think a note on NEWS might be better fort this specific > usage. Okay, I will remove the second #warning and add this to NEWS: * The Linux-specific <sys/sysctl.h> header and the sysctl function have been deprecated and will be removed from a future version of glibc. Application should directly access /proc instead. For obtaining random bits, the getentropy function can be used. >>> What about a link warning stating this interface is deprecated and it would be >>> removed? My idea is to warn when people are static linking as well. >> >> They already get two warnings (from the header and the call site). Do >> we need a third warning? It's only helpful if you don't use the header, >> I think, but even with Go's cgo, you'll see the GCC warnings. > > Fair enough. > > Patch LGTM, thanks. > > Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> Thanks! Florian
diff --git a/NEWS b/NEWS index 00f9e855a2..fffff79576 100644 --- a/NEWS +++ b/NEWS @@ -61,6 +61,9 @@ Deprecated and removed features, and other changes affecting compatibility: * On 32-bit Arm, support for the port-based I/O emulation and the <sys/io.h> header have been removed. +* The Linux-specific <sys/sysctl.h> header and the sysctl function have been + deprecated and will be removed from a future version of glibc. + Changes to build and runtime requirements: * GCC 6.2 or later is required to build the GNU C Library. diff --git a/include/sys/sysctl.h b/include/sys/sysctl.h index 2a15e91354..fa102aa226 100644 --- a/include/sys/sysctl.h +++ b/include/sys/sysctl.h @@ -1,13 +1,3 @@ #ifndef _SYS_SYSCTL_H #include_next <sys/sysctl.h> - -# ifndef _ISOMAC - -/* Read or write system parameters (Linux, FreeBSD specific). */ -extern int __sysctl (int *__name, int __nlen, void *__oldval, - size_t *__oldlenp, void *__newval, size_t __newlen); -libc_hidden_proto (__sysctl) - - -# endif /* !_ISOMAC */ #endif /* _SYS_SYSCTL_H */ diff --git a/scripts/check-installed-headers.sh b/scripts/check-installed-headers.sh index e4f37d33f8..ef6ed94a2e 100644 --- a/scripts/check-installed-headers.sh +++ b/scripts/check-installed-headers.sh @@ -53,7 +53,6 @@ trap "rm -f '$cih_test_c'" 0 failed=0 is_x86_64=unknown -is_x32=unknown for header in "$@"; do # Skip various headers for which this test gets a false failure. case "$header" in @@ -75,27 +74,10 @@ for header in "$@"; do (finclude/*) continue;; - # sys/sysctl.h is unsupported for x32. + # sys/sysctl.h produces a deprecation warning and therefore + # fails compilation with -Werror. (sys/sysctl.h) - case "$is_x32" in - (yes) continue;; - (no) ;; - (unknown) - cat >"$cih_test_c" <<EOF -#if defined __x86_64__ && defined __ILP32__ -# error "is x32" -#endif -EOF - if $cc_cmd -fsyntax-only "$cih_test_c" > /dev/null 2>&1 - then - is_x32=no - else - is_x32=yes - continue - fi - ;; - esac - ;; + continue;; # sys/vm86.h is "unsupported on x86-64" and errors out on that target. (sys/vm86.h) diff --git a/sysdeps/unix/sysv/linux/sys/sysctl.h b/sysdeps/unix/sysv/linux/sys/sysctl.h index 0f6e71ba7e..15ee9642ab 100644 --- a/sysdeps/unix/sysv/linux/sys/sysctl.h +++ b/sysdeps/unix/sysv/linux/sys/sysctl.h @@ -18,6 +18,9 @@ #ifndef _SYS_SYSCTL_H #define _SYS_SYSCTL_H 1 +#warning "The <sys/sysctl.h> header is deprecated and will be removed." +#warning "Directly access the /proc file system instead." + #include <features.h> #define __need_size_t #include <stddef.h> @@ -66,7 +69,8 @@ __BEGIN_DECLS /* Read or write system parameters. */ extern int sysctl (int *__name, int __nlen, void *__oldval, - size_t *__oldlenp, void *__newval, size_t __newlen) __THROW; + size_t *__oldlenp, void *__newval, size_t __newlen) __THROW + __attribute_deprecated__; __END_DECLS diff --git a/sysdeps/unix/sysv/linux/sysctl.c b/sysdeps/unix/sysv/linux/sysctl.c index 33afdd918b..0f18c69abe 100644 --- a/sysdeps/unix/sysv/linux/sysctl.c +++ b/sysdeps/unix/sysv/linux/sysctl.c @@ -17,8 +17,7 @@ <http://www.gnu.org/licenses/>. */ #include <errno.h> -#include <string.h> /* For the real memset prototype. */ -#include <sys/sysctl.h> +#include <linux/sysctl.h> #include <sysdep.h> #include <sys/syscall.h> @@ -39,5 +38,4 @@ __sysctl (int *name, int nlen, void *oldval, size_t *oldlenp, return INLINE_SYSCALL (_sysctl, 1, &args); } -libc_hidden_def (__sysctl) weak_alias (__sysctl, sysctl)