This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On Sat, Dec 30, 2017 at 06:09:22PM -0500, Luke Shumaker wrote: > Dmitry V. Levin wrote: > > It would be quite logical for tst-ttyname to return EXIT_UNSUPPORTED > > as soon as support_become_root failed. > > Indeed; in the original version of the test, whereit called > support_become_root, it said: > > support_become_root (); > > if (unshare (CLONE_NEWNS) < 0) > FAIL_UNSUPPORTED ("could not enter new mount namespace"); > > Where, that call to `unshare (CLONE_NEWNS)` is guaranteed to to fail > if `support_become_root ()` failed; it would have been pointless to > check for failure twice in the same spot by directly checking > support_become_root. > > However, when Florian cleaned up the test, the call to > support_become_root got moved to be much earlier in the test; my above > assumtion that we would immediately be doing something that would > implicitly check the success of support_become_root became invalid. > Of course, I didn't realize that when I said "LGTM" To Florian's > patch. > > If the only problem with the test is that support_become_root fails, > then we can count on the later > > if (!support_enter_mount_namespace ()) > FAIL_UNSUPPORTED ("could not enter new mount namespace"); > > causing us to exit with EXIT_UNSUPPORTED. But, checking the result of > support_become_root allows us to exit earlier; saving pointless work. > > On Tue, 26 Dec 2017 09:10:02 -0500, > Dmitry V. Levin wrote: > > diff --git a/sysdeps/unix/sysv/linux/tst-ttyname.c b/sysdeps/unix/sysv/linux/tst-ttyname.c > > index 0fdf1a8..3943403 100644 > > --- a/sysdeps/unix/sysv/linux/tst-ttyname.c > > +++ b/sysdeps/unix/sysv/linux/tst-ttyname.c > > @@ -554,7 +554,8 @@ run_chroot_tests (const char *slavename, int slave) > > static int > > do_test (void) > > { > > - support_become_root (); > > + if (!support_become_root ()) > > + return EXIT_UNSUPPORTED; > > > > int ret1 = do_in_chroot_1 (run_chroot_tests); > > if (ret1 == EXIT_UNSUPPORTED) > > So, with the above in mind, I do think this patch should be applied. > > But, I do agree with Florian when he said > > Florian Weimer wrote: > > I don't think the glibc test suite is supposed to pass in such an > > environment [without ptmx]. If you don't provide /dev/null, /sys, > > or /proc to the tests, some of them will fail as well. I still > > think that the current test accurately reflects the inadequacy of > > your test environment. > > So I don't nescessarily think that this should be backported to the > 2.26 stable branch. If a backported test introduced a regression in the test suite, then a fix has to be backported as well, I don't think anybody argues with that. This is exactly the case - tst-ttyname was backported to the stable branch and caused a regression in the test suite there. One cannot say that environments without ptmx are not supported by 2.26 - the fact is that they were definitely supported at 2.26 release time. Also, one cannot declare these environments unsupported in master branch without reaching a consensus. tst-ttyname was not purposefully made to fail when ptmx is not available - it's just an omission that has to be fixed. -- ldv
Attachment:
signature.asc
Description: PGP signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |