| Message ID | 20220714124611.9772-1-akumar@suse.de |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] open06.c: convert to new LTP API | expand |
Hi Avinesh, > - if (mknod(fname, S_IFIFO | 0644, 0) == -1) > - tst_brkm(TBROK, cleanup, "mknod FAILED"); > + SAFE_MKFIFO(TEMP_FIFO, 0644); You changed test from mknod() to mkfifo(). May I know why? It would be worth to mention the reason in the commit message. Should not we keep mknod() ? SAFE_MKNOD(TEMP_FIFO, S_IFIFO | 0644, 0); According to man mknod(2) your change is correct: POSIX.1-2001 says: "The only portable use of mknod() is to create a FIFO-special file. If mode is not S_IFIFO or dev is not 0, the behavior of mknod() is unspecified." However, nowadays one should never use mknod() for this purpose; one should use mkfifo(3), a function especially defined for this purpose. Also note LTP tests should test even deprecated kernel API, we *might* want to test both mkfifo() and mknod() via .test_variants. But I'd like to hear the input of the others, because both glibc and musl use SYS_mknod or SYS_mknodat for mkfifo() implementation with very thin wrapper, thus not sure if it's worth to test also mknod(). Kind regards, Petr
Hi Petr, On Thursday, July 14, 2022 6:48:48 PM IST Petr Vorel wrote: > Hi Avinesh, > > > - if (mknod(fname, S_IFIFO | 0644, 0) == -1) > > - tst_brkm(TBROK, cleanup, "mknod FAILED"); > > + SAFE_MKFIFO(TEMP_FIFO, 0644); > You changed test from mknod() to mkfifo(). May I know why? > It would be worth to mention the reason in the commit message. > > Should not we keep mknod() ? > SAFE_MKNOD(TEMP_FIFO, S_IFIFO | 0644, 0); > > According to man mknod(2) your change is correct: > > POSIX.1-2001 says: "The only portable use of mknod() is to create > a FIFO-special file. If mode is not S_IFIFO or dev is not 0, the > behavior of mknod() is unspecified." However, nowadays one > should never use mknod() for this purpose; one should use > mkfifo(3), a function especially defined for this purpose. > > Also note LTP tests should test even deprecated kernel API, we *might* want to > test both mkfifo() and mknod() via .test_variants. But I'd like to hear the > input of the others, because both glibc and musl use SYS_mknod or SYS_mknodat > for mkfifo() implementation with very thin wrapper, thus not sure if it's worth > to test also mknod(). I changed to SAFE_MKFIFO as it seemed more intuitive in this open() test, but yes, I should have mentioned it in the commit message. I have not checked the mkfifo() implementation in libraries, so please lmk which one to proceed with here, I will send updated version if needed. > > Kind regards, > Petr > Thanks, Avinesh
Hi all, > Hi Petr, > On Thursday, July 14, 2022 6:48:48 PM IST Petr Vorel wrote: > > Hi Avinesh, > > > - if (mknod(fname, S_IFIFO | 0644, 0) == -1) > > > - tst_brkm(TBROK, cleanup, "mknod FAILED"); > > > + SAFE_MKFIFO(TEMP_FIFO, 0644); > > You changed test from mknod() to mkfifo(). May I know why? > > It would be worth to mention the reason in the commit message. > > Should not we keep mknod() ? > > SAFE_MKNOD(TEMP_FIFO, S_IFIFO | 0644, 0); > > According to man mknod(2) your change is correct: > > POSIX.1-2001 says: "The only portable use of mknod() is to create > > a FIFO-special file. If mode is not S_IFIFO or dev is not 0, the > > behavior of mknod() is unspecified." However, nowadays one > > should never use mknod() for this purpose; one should use > > mkfifo(3), a function especially defined for this purpose. > > Also note LTP tests should test even deprecated kernel API, we *might* want to > > test both mkfifo() and mknod() via .test_variants. But I'd like to hear the > > input of the others, because both glibc and musl use SYS_mknod or SYS_mknodat > > for mkfifo() implementation with very thin wrapper, thus not sure if it's worth > > to test also mknod(). > I changed to SAFE_MKFIFO as it seemed more intuitive in this open() test, > but yes, I should have mentioned it in the commit message. > I have not checked the mkfifo() implementation in libraries, so please > lmk which one to proceed with here, I will send updated version if needed. Thinking about it twice, given mknod() / mkfifo() are used in the setup it does not look worth to run .test_variants just for this. So, unless anybody disagrees, it's ok to keep SAFE_MKFIFO(). I'll wait little longer before merging it (with note of changed function in the commit message). Kind regards, Petr > > Kind regards, > > Petr > Thanks, > Avinesh
diff --git a/testcases/kernel/syscalls/open/open06.c b/testcases/kernel/syscalls/open/open06.c index 6c774ce84..a3c3bcb89 100644 --- a/testcases/kernel/syscalls/open/open06.c +++ b/testcases/kernel/syscalls/open/open06.c @@ -1,90 +1,34 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + /* - * * Copyright (c) International Business Machines Corp., 2001 - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program 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 General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * Copyright (c) 2022 SUSE LLC Avinesh Kumar <avinesh.kumar@suse.com> */ -/* - * DESCRIPTION - * Testcase to check open(2) sets errno to ENXIO correctly. +/*\ + * [Description] * - * ALGORITHM - * Create a named pipe using mknod(2). Attempt to - * open(2) the pipe for writing. The open(2) should - * fail with ENXIO. + * Verify that open(2) fails with ENXIO when + * O_NONBLOCK | O_WRONLY is set, the named file is a FIFO, + * and no process has the FIFO open for reading. */ -#include <sys/types.h> -#include <sys/stat.h> -#include <fcntl.h> -#include <errno.h> -#include <unistd.h> -#include "test.h" - -char *TCID = "open06"; -int TST_TOTAL = 1; - -static void setup(void); -static void cleanup(void); - -static char fname[100] = "fifo"; - -int main(int ac, char **av) -{ - int lc; - tst_parse_opts(ac, av, NULL, NULL); +#include "tst_test.h" - setup(); - - for (lc = 0; TEST_LOOPING(lc); lc++) { - tst_count = 0; - - TEST(open(fname, O_NONBLOCK | O_WRONLY)); - if (TEST_RETURN != -1) { - tst_resm(TFAIL, "open(2) succeeded unexpectedly"); - continue; - } - - if (TEST_ERRNO != ENXIO) - tst_resm(TFAIL, "Expected ENXIO got %d", TEST_ERRNO); - else - tst_resm(TPASS, "call returned expected ENXIO error"); - } - - cleanup(); - tst_exit(); -} +#define TEMP_FIFO "tmpfile" static void setup(void) { - tst_sig(NOFORK, DEF_HANDLER, cleanup); - - TEST_PAUSE; - - tst_tmpdir(); - - sprintf(fname, "%s.%d", fname, getpid()); - - if (mknod(fname, S_IFIFO | 0644, 0) == -1) - tst_brkm(TBROK, cleanup, "mknod FAILED"); + SAFE_MKFIFO(TEMP_FIFO, 0644); } -static void cleanup(void) +static void run(void) { - unlink(fname); - - tst_rmdir(); + TST_EXP_FAIL2(open(TEMP_FIFO, O_NONBLOCK | O_WRONLY), ENXIO); } + +static struct tst_test test = { + .test_all = run, + .setup = setup, + .needs_tmpdir = 1 +};
Signed-off-by: Avinesh Kumar <akumar@suse.de> --- testcases/kernel/syscalls/open/open06.c | 94 +++++-------------------- 1 file changed, 19 insertions(+), 75 deletions(-)