This is the mail archive of the 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]

Re: [PATCH v3] tst-ttyname: skip the test if failed to become root

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

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

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.

Happy hacking,
~ Luke Shumaker

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]