| Message ID | 20171225214154.GB11045@altlinux.org |
|---|---|
| State | New, archived |
| Headers |
Received: (qmail 19724 invoked by alias); 25 Dec 2017 21:41:58 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 19670 invoked by uid 89); 25 Dec 2017 21:41:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: vmicros1.altlinux.org Date: Tue, 26 Dec 2017 00:41:54 +0300 From: "Dmitry V. Levin" <ldv@altlinux.org> To: libc-alpha@sourceware.org Subject: [PATCH v2] tst-ttyname: skip the test when /dev/ptmx is not available Message-ID: <20171225214154.GB11045@altlinux.org> Mail-Followup-To: libc-alpha@sourceware.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline |
Commit Message
Dmitry V. Levin
Dec. 25, 2017, 9:41 p.m. UTC
* sysdeps/unix/sysv/linux/tst-ttyname.c (do_in_chroot_1): Skip the test instead of failing in case of ENOENT returned by posix_openpt. --- ChangeLog | 5 +++++ sysdeps/unix/sysv/linux/tst-ttyname.c | 9 ++++++++- 2 files changed, 13 insertions(+), 1 deletion(-)
Comments
On Mon, 25 Dec 2017 16:41:54 -0500, Dmitry V. Levin wrote: > > * sysdeps/unix/sysv/linux/tst-ttyname.c (do_in_chroot_1): Skip the > test instead of failing in case of ENOENT returned by posix_openpt. > --- > ChangeLog | 5 +++++ > sysdeps/unix/sysv/linux/tst-ttyname.c | 9 ++++++++- > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/sysdeps/unix/sysv/linux/tst-ttyname.c b/sysdeps/unix/sysv/linux/tst-ttyname.c > index 0fdf1a8..6848a6d 100644 > --- a/sysdeps/unix/sysv/linux/tst-ttyname.c > +++ b/sysdeps/unix/sysv/linux/tst-ttyname.c > @@ -253,7 +253,14 @@ do_in_chroot_1 (int (*cb)(const char *, int)) > /* Open the PTS that we'll be testing on. */ > int master; > char *slavename; > - VERIFY ((master = posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK)) >= 0); > + master = posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK); > + if (master < 0) > + { > + if (errno == ENOENT) > + FAIL_UNSUPPORTED ("posix_openpt: %m"); > + else > + FAIL_EXIT1 ("posix_openpt: %m"); > + } > VERIFY ((slavename = ptsname (master))); > VERIFY (unlockpt (master) == 0); > if (strncmp (slavename, "/dev/pts/", 9) != 0) > -- > ldv As I explained in a different thread: I generally support applying this and backporting it to the 2.26 release branch. However, shouldn't it also make this same change in do_in_chroot_2? Of course, if do_in_chroot_1 returns EXIT_UNSUPPORTED, then do_in_chroot_2 doesn't get to run. But, as I think about that more, I don't think that's the correct thing to do. Maybe also do this? @@ -557,12 +571,7 @@ do_test (void) support_become_root (); int ret1 = do_in_chroot_1 (run_chroot_tests); - if (ret1 == EXIT_UNSUPPORTED) - return ret1; - int ret2 = do_in_chroot_2 (run_chroot_tests); - if (ret2 == EXIT_UNSUPPORTED) - return ret2; return ret1 | ret2;
On Tue, Jan 02, 2018 at 12:46:29AM -0500, Luke Shumaker wrote: > On Mon, 25 Dec 2017 16:41:54 -0500, Dmitry V. Levin wrote: > > > > * sysdeps/unix/sysv/linux/tst-ttyname.c (do_in_chroot_1): Skip the > > test instead of failing in case of ENOENT returned by posix_openpt. > > --- > > ChangeLog | 5 +++++ > > sysdeps/unix/sysv/linux/tst-ttyname.c | 9 ++++++++- > > 2 files changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/sysdeps/unix/sysv/linux/tst-ttyname.c b/sysdeps/unix/sysv/linux/tst-ttyname.c > > index 0fdf1a8..6848a6d 100644 > > --- a/sysdeps/unix/sysv/linux/tst-ttyname.c > > +++ b/sysdeps/unix/sysv/linux/tst-ttyname.c > > @@ -253,7 +253,14 @@ do_in_chroot_1 (int (*cb)(const char *, int)) > > /* Open the PTS that we'll be testing on. */ > > int master; > > char *slavename; > > - VERIFY ((master = posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK)) >= 0); > > + master = posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK); > > + if (master < 0) > > + { > > + if (errno == ENOENT) > > + FAIL_UNSUPPORTED ("posix_openpt: %m"); > > + else > > + FAIL_EXIT1 ("posix_openpt: %m"); > > + } > > VERIFY ((slavename = ptsname (master))); > > VERIFY (unlockpt (master) == 0); > > if (strncmp (slavename, "/dev/pts/", 9) != 0) > > -- > > ldv > > As I explained in a different thread: I generally support applying > this and backporting it to the 2.26 release branch. > > However, shouldn't it also make this same change in do_in_chroot_2? If the first posix_openpt invocation succeeded, we may expect all subsequent posix_openpt invocations will succeed, too. If they don't, there must be something odd going on.
On Tue, Dec 26, 2017 at 12:41:54AM +0300, Dmitry V. Levin wrote: > * sysdeps/unix/sysv/linux/tst-ttyname.c (do_in_chroot_1): Skip the > test instead of failing in case of ENOENT returned by posix_openpt. > --- > ChangeLog | 5 +++++ > sysdeps/unix/sysv/linux/tst-ttyname.c | 9 ++++++++- > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/sysdeps/unix/sysv/linux/tst-ttyname.c b/sysdeps/unix/sysv/linux/tst-ttyname.c > index 0fdf1a8..6848a6d 100644 > --- a/sysdeps/unix/sysv/linux/tst-ttyname.c > +++ b/sysdeps/unix/sysv/linux/tst-ttyname.c > @@ -253,7 +253,14 @@ do_in_chroot_1 (int (*cb)(const char *, int)) > /* Open the PTS that we'll be testing on. */ > int master; > char *slavename; > - VERIFY ((master = posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK)) >= 0); > + master = posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK); > + if (master < 0) > + { > + if (errno == ENOENT) > + FAIL_UNSUPPORTED ("posix_openpt: %m"); > + else > + FAIL_EXIT1 ("posix_openpt: %m"); > + } > VERIFY ((slavename = ptsname (master))); > VERIFY (unlockpt (master) == 0); > if (strncmp (slavename, "/dev/pts/", 9) != 0) Florian, do you have any objections to this fix?
* Dmitry V. Levin: > On Tue, Dec 26, 2017 at 12:41:54AM +0300, Dmitry V. Levin wrote: >> * sysdeps/unix/sysv/linux/tst-ttyname.c (do_in_chroot_1): Skip the >> test instead of failing in case of ENOENT returned by posix_openpt. >> --- >> ChangeLog | 5 +++++ >> sysdeps/unix/sysv/linux/tst-ttyname.c | 9 ++++++++- >> 2 files changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/sysdeps/unix/sysv/linux/tst-ttyname.c b/sysdeps/unix/sysv/linux/tst-ttyname.c >> index 0fdf1a8..6848a6d 100644 >> --- a/sysdeps/unix/sysv/linux/tst-ttyname.c >> +++ b/sysdeps/unix/sysv/linux/tst-ttyname.c >> @@ -253,7 +253,14 @@ do_in_chroot_1 (int (*cb)(const char *, int)) >> /* Open the PTS that we'll be testing on. */ >> int master; >> char *slavename; >> - VERIFY ((master = posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK)) >= 0); >> + master = posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK); >> + if (master < 0) >> + { >> + if (errno == ENOENT) >> + FAIL_UNSUPPORTED ("posix_openpt: %m"); >> + else >> + FAIL_EXIT1 ("posix_openpt: %m"); >> + } >> VERIFY ((slavename = ptsname (master))); >> VERIFY (unlockpt (master) == 0); >> if (strncmp (slavename, "/dev/pts/", 9) != 0) > > Florian, do you have any objections to this fix? I still do not see the point, consdering that a test for posix_openpt would still fail, had we one, but I don't have objections to the change above.
diff --git a/sysdeps/unix/sysv/linux/tst-ttyname.c b/sysdeps/unix/sysv/linux/tst-ttyname.c index 0fdf1a8..6848a6d 100644 --- a/sysdeps/unix/sysv/linux/tst-ttyname.c +++ b/sysdeps/unix/sysv/linux/tst-ttyname.c @@ -253,7 +253,14 @@ do_in_chroot_1 (int (*cb)(const char *, int)) /* Open the PTS that we'll be testing on. */ int master; char *slavename; - VERIFY ((master = posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK)) >= 0); + master = posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK); + if (master < 0) + { + if (errno == ENOENT) + FAIL_UNSUPPORTED ("posix_openpt: %m"); + else + FAIL_EXIT1 ("posix_openpt: %m"); + } VERIFY ((slavename = ptsname (master))); VERIFY (unlockpt (master) == 0); if (strncmp (slavename, "/dev/pts/", 9) != 0)