Message ID | 20171019183228.4BD2A428666AA@oldenburg.str.redhat.com |
---|---|
State | New |
Headers | show |
Series | sysconf: Fix missing definition of UIO_MAXIOV on Linux [BZ #22321] | expand |
On 10/19/2017 11:32 AM, Florian Weimer wrote: > 2017-10-19 Florian Weimer <fweimer@redhat.com> > > [BZ #22321] > sysconf: Fix missing definition of UIO_MAXIOV on Linux. > * sysdeps/posix/sysconf.c: Include <sys/uio.h>. > * sysdeps/unix/sysv/linux/Makefile (tests): Add tst-sysconf-iov_max. > (tst-sysconf-iov_max): Link with tst-sysconf-iov_max-uapi.o. > * sysdeps/unix/sysv/linux/tst-sysconf-iov_max.c: New file. > * sysdeps/unix/sysv/linux/tst-sysconf-iov_max-uapi.c: Likewise. Looks good to me. I had some questions though, see below. OK to commit if the answer to the questions is "Yes." and we add additional tests. Reviewed-by: Carlos O'Donell <carlos@redhat.com> > diff --git a/sysdeps/posix/sysconf.c b/sysdeps/posix/sysconf.c > index a95e1b3f05..254f87c437 100644 > --- a/sysdeps/posix/sysconf.c > +++ b/sysdeps/posix/sysconf.c > @@ -29,6 +29,7 @@ > #include <sys/stat.h> > #include <sys/sysinfo.h> > #include <sys/types.h> > +#include <sys/uio.h> OK. Inclusion of define enables returned result. > #include <regex.h> > > #define NEED_SPEC_ARRAY 0 > diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile > index 30bd167bae..0c8a009b5e 100644 > --- a/sysdeps/unix/sysv/linux/Makefile > +++ b/sysdeps/unix/sysv/linux/Makefile > @@ -50,7 +50,7 @@ sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \ > bits/siginfo-arch.h bits/siginfo-consts-arch.h > > tests += tst-clone tst-clone2 tst-clone3 tst-fanotify tst-personality \ > - tst-quota tst-sync_file_range test-errno-linux > + tst-quota tst-sync_file_range test-errno-linux tst-sysconf-iov_max OK. Thanks for the new test. > > # Generate the list of SYS_* macros for the system calls (__NR_* > # macros). The file syscall-names.list contains all possible system > @@ -95,6 +95,9 @@ $(objpfx)tst-syscall-list.out: \ > $(objpfx)tst-syscall-list-sys.list > $(BASH) $^ $(AWK) > $@; $(evaluate-test) > > +# Separate object file for access to the constant from the UAPI header. > +$(objpfx)tst-sysconf-iov_max: $(objpfx)tst-sysconf-iov_max-uapi.o > + > endif # $(subdir) == misc > > ifeq ($(subdir),time) > diff --git a/sysdeps/unix/sysv/linux/tst-sysconf-iov_max-uapi.c b/sysdeps/unix/sysv/linux/tst-sysconf-iov_max-uapi.c > new file mode 100644 > index 0000000000..1240b846e6 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/tst-sysconf-iov_max-uapi.c > @@ -0,0 +1,27 @@ > +/* Check IOV_MAX definition: Helper function to capture UAPI header value. > + Copyright (C) 2017 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/>. */ > + > +/* Use a separate function to avoid header compatibility issues. */ OK. Clever use of the distinct object file. > + > +#include <linux/uio.h> > + > +long > +uio_maxiov_value (void) > +{ > + return UIO_MAXIOV; > +} > diff --git a/sysdeps/unix/sysv/linux/tst-sysconf-iov_max.c b/sysdeps/unix/sysv/linux/tst-sysconf-iov_max.c > new file mode 100644 > index 0000000000..e85bd59550 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/tst-sysconf-iov_max.c > @@ -0,0 +1,38 @@ > +/* Check IOV_MAX definition for consistency (bug 22321). > + Copyright (C) 2017 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/>. */ > + > +/* Defined in tst-sysconf-iov_max-uapi.c. */ > +long uio_maxiov_value (void); > + > + > +#include <limits.h> > +#include <support/check.h> > +#include <sys/uio.h> > +#include <unistd.h> > + > +static int > +do_test (void) > +{ > + TEST_VERIFY (IOV_MAX == uio_maxiov_value ()); > + TEST_VERIFY (UIO_MAXIOV == uio_maxiov_value ()); > + TEST_VERIFY (sysconf (_SC_UIO_MAXIOV) == uio_maxiov_value ()); > + TEST_VERIFY (sysconf (_SC_IOV_MAX) == uio_maxiov_value ()); Is there an expectation that the value, although consistent, should not be -1? e.g. TEST_VERIFY (IOV_MAX != -1); TEST_VERIFY (UIOV_MAX != -1); Do we need to verify it is >= _XOPEN_IOV_MAX? e.g. TEST_VERIFY (IOV_MAX >= _XOPEN_IOV_MAX); TEST_VERIFY (UIOV_MAX >= _XOPEN_IOV_MAX); > + return 0; > +} > + > +#include <support/test-driver.c> >
On 10/19/2017 09:13 PM, Carlos O'Donell wrote: >> + TEST_VERIFY (IOV_MAX == uio_maxiov_value ()); >> + TEST_VERIFY (UIO_MAXIOV == uio_maxiov_value ()); >> + TEST_VERIFY (sysconf (_SC_UIO_MAXIOV) == uio_maxiov_value ()); >> + TEST_VERIFY (sysconf (_SC_IOV_MAX) == uio_maxiov_value ()); > Is there an expectation that the value, although consistent, > should not be -1? > > e.g. > TEST_VERIFY (IOV_MAX != -1); > TEST_VERIFY (UIOV_MAX != -1); > > Do we need to verify it is >= _XOPEN_IOV_MAX? > > e.g. > TEST_VERIFY (IOV_MAX >= _XOPEN_IOV_MAX); > TEST_VERIFY (UIOV_MAX >= _XOPEN_IOV_MAX); Should I add TEST_VERIFY (uio_maxiov_value () >= _XOPEN_IOV_MAX); ? That would indirectly cover all the additional tests you suggest. Thanks, Florian
On 10/19/2017 12:21 PM, Florian Weimer wrote: > On 10/19/2017 09:13 PM, Carlos O'Donell wrote: >>> + TEST_VERIFY (IOV_MAX == uio_maxiov_value ()); >>> + TEST_VERIFY (UIO_MAXIOV == uio_maxiov_value ()); >>> + TEST_VERIFY (sysconf (_SC_UIO_MAXIOV) == uio_maxiov_value ()); >>> + TEST_VERIFY (sysconf (_SC_IOV_MAX) == uio_maxiov_value ()); > >> Is there an expectation that the value, although consistent, >> should not be -1? >> >> e.g. >> TEST_VERIFY (IOV_MAX != -1); >> TEST_VERIFY (UIOV_MAX != -1); >> >> Do we need to verify it is >= _XOPEN_IOV_MAX? >> >> e.g. >> TEST_VERIFY (IOV_MAX >= _XOPEN_IOV_MAX); >> TEST_VERIFY (UIOV_MAX >= _XOPEN_IOV_MAX); > > Should I add > > TEST_VERIFY (uio_maxiov_value () >= _XOPEN_IOV_MAX); > > ? > > That would indirectly cover all the additional tests you suggest. ... unless _XOPEN_IOV_MAX is incorrectly defined to -1? :-)
diff --git a/sysdeps/posix/sysconf.c b/sysdeps/posix/sysconf.c index a95e1b3f05..254f87c437 100644 --- a/sysdeps/posix/sysconf.c +++ b/sysdeps/posix/sysconf.c @@ -29,6 +29,7 @@ #include <sys/stat.h> #include <sys/sysinfo.h> #include <sys/types.h> +#include <sys/uio.h> #include <regex.h> #define NEED_SPEC_ARRAY 0 diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile index 30bd167bae..0c8a009b5e 100644 --- a/sysdeps/unix/sysv/linux/Makefile +++ b/sysdeps/unix/sysv/linux/Makefile @@ -50,7 +50,7 @@ sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \ bits/siginfo-arch.h bits/siginfo-consts-arch.h tests += tst-clone tst-clone2 tst-clone3 tst-fanotify tst-personality \ - tst-quota tst-sync_file_range test-errno-linux + tst-quota tst-sync_file_range test-errno-linux tst-sysconf-iov_max # Generate the list of SYS_* macros for the system calls (__NR_* # macros). The file syscall-names.list contains all possible system @@ -95,6 +95,9 @@ $(objpfx)tst-syscall-list.out: \ $(objpfx)tst-syscall-list-sys.list $(BASH) $^ $(AWK) > $@; $(evaluate-test) +# Separate object file for access to the constant from the UAPI header. +$(objpfx)tst-sysconf-iov_max: $(objpfx)tst-sysconf-iov_max-uapi.o + endif # $(subdir) == misc ifeq ($(subdir),time) diff --git a/sysdeps/unix/sysv/linux/tst-sysconf-iov_max-uapi.c b/sysdeps/unix/sysv/linux/tst-sysconf-iov_max-uapi.c new file mode 100644 index 0000000000..1240b846e6 --- /dev/null +++ b/sysdeps/unix/sysv/linux/tst-sysconf-iov_max-uapi.c @@ -0,0 +1,27 @@ +/* Check IOV_MAX definition: Helper function to capture UAPI header value. + Copyright (C) 2017 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/>. */ + +/* Use a separate function to avoid header compatibility issues. */ + +#include <linux/uio.h> + +long +uio_maxiov_value (void) +{ + return UIO_MAXIOV; +} diff --git a/sysdeps/unix/sysv/linux/tst-sysconf-iov_max.c b/sysdeps/unix/sysv/linux/tst-sysconf-iov_max.c new file mode 100644 index 0000000000..e85bd59550 --- /dev/null +++ b/sysdeps/unix/sysv/linux/tst-sysconf-iov_max.c @@ -0,0 +1,38 @@ +/* Check IOV_MAX definition for consistency (bug 22321). + Copyright (C) 2017 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/>. */ + +/* Defined in tst-sysconf-iov_max-uapi.c. */ +long uio_maxiov_value (void); + + +#include <limits.h> +#include <support/check.h> +#include <sys/uio.h> +#include <unistd.h> + +static int +do_test (void) +{ + TEST_VERIFY (IOV_MAX == uio_maxiov_value ()); + TEST_VERIFY (UIO_MAXIOV == uio_maxiov_value ()); + TEST_VERIFY (sysconf (_SC_UIO_MAXIOV) == uio_maxiov_value ()); + TEST_VERIFY (sysconf (_SC_IOV_MAX) == uio_maxiov_value ()); + return 0; +} + +#include <support/test-driver.c>