| Message ID | 20231120154248.15048-1-andrea.cervesato@suse.de |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [v1] Delete fork09 test | expand |
Hi Andrea, > From: Andrea Cervesato <andrea.cervesato@suse.com> > The fork09 test is actually testing what fork10 is already testing: > accessing an open child's file from parent. For this reason, we delete > it and let fork10 doing its job. in fork09 parent opens maximum number of files, child closes one and attempts to open another. That is not exactly the same, but it's very similar. I'm not sure if the scenario would really tests different code path in kernel/libc or not and we can happily delete it. Cyril, WDYT? Kind regards, Petr
Hi all, On 2023-11-28 13:56, Petr Vorel wrote: > Hi Andrea, > >> From: Andrea Cervesato <andrea.cervesato@suse.com> > >> The fork09 test is actually testing what fork10 is already testing: >> accessing an open child's file from parent. For this reason, we delete >> it and let fork10 doing its job. > > in fork09 parent opens maximum number of files, child closes one and > attempts to > open another. That is not exactly the same, but it's very similar. > > I'm not sure if the scenario would really tests different code path in > kernel/libc or not and we can happily delete it. > Cyril, WDYT? @Li, WDYT? > > Kind regards, > Petr
On Tue, Dec 19, 2023 at 10:55 PM pvorel <pvorel@suse.de> wrote: > Hi all, > > On 2023-11-28 13:56, Petr Vorel wrote: > > Hi Andrea, > > > >> From: Andrea Cervesato <andrea.cervesato@suse.com> > > > >> The fork09 test is actually testing what fork10 is already testing: > >> accessing an open child's file from parent. For this reason, we delete > >> it and let fork10 doing its job. > > > > in fork09 parent opens maximum number of files, child closes one and > > attempts to > > open another. That is not exactly the same, but it's very similar. > > > > I'm not sure if the scenario would really tests different code path in > > kernel/libc or not and we can happily delete it. > > Cyril, WDYT? > > @Li, WDYT? > Petr, I think you're right, they're not the same test. In fork10 parent tries to verify the contents of the file that reset the file offset pointer by child. But for09 is a bounder test to verify if the child can open one more file when 'fd' number approaches (OpenMax-1).
> On Tue, Dec 19, 2023 at 10:55 PM pvorel <pvorel@suse.de> wrote: > > Hi all, > > On 2023-11-28 13:56, Petr Vorel wrote: > > > Hi Andrea, > > >> From: Andrea Cervesato <andrea.cervesato@suse.com> > > >> The fork09 test is actually testing what fork10 is already testing: > > >> accessing an open child's file from parent. For this reason, we delete > > >> it and let fork10 doing its job. > > > in fork09 parent opens maximum number of files, child closes one and > > > attempts to > > > open another. That is not exactly the same, but it's very similar. > > > I'm not sure if the scenario would really tests different code path in > > > kernel/libc or not and we can happily delete it. > > > Cyril, WDYT? > > @Li, WDYT? > Petr, I think you're right, they're not the same test. > In fork10 parent tries to verify the contents of the file that reset the > file offset pointer by child. > But for09 is a bounder test to verify if the child can open one more file > when 'fd' number approaches (OpenMax-1). Thank you! Closing this as rejected, Andrea feel free to rewrite the test as well. Kind regards, Petr
Hi! On 12/20/23 13:29, Petr Vorel wrote: >> On Tue, Dec 19, 2023 at 10:55 PM pvorel <pvorel@suse.de> wrote: >>> Hi all, >>> On 2023-11-28 13:56, Petr Vorel wrote: >>>> Hi Andrea, >>>>> From: Andrea Cervesato <andrea.cervesato@suse.com> >>>>> The fork09 test is actually testing what fork10 is already testing: >>>>> accessing an open child's file from parent. For this reason, we delete >>>>> it and let fork10 doing its job. >>>> in fork09 parent opens maximum number of files, child closes one and >>>> attempts to >>>> open another. That is not exactly the same, but it's very similar. >>>> I'm not sure if the scenario would really tests different code path in >>>> kernel/libc or not and we can happily delete it. >>>> Cyril, WDYT? >>> @Li, WDYT? > >> Petr, I think you're right, they're not the same test. >> In fork10 parent tries to verify the contents of the file that reset the >> file offset pointer by child. >> But for09 is a bounder test to verify if the child can open one more file >> when 'fd' number approaches (OpenMax-1). > Thank you! Closing this as rejected, Andrea feel free to rewrite the test as > well. The reason why this test should be deleted is contained in the following discussion, but I can rewrite it if it's strictly necessary. https://patchwork.ozlabs.org/project/ltp/patch/20230907150538.16772-1-andrea.cervesato@suse.de/ > Kind regards, > Petr Andrea
diff --git a/runtest/syscalls b/runtest/syscalls index c98992d44..430d32e6a 100644 --- a/runtest/syscalls +++ b/runtest/syscalls @@ -383,7 +383,6 @@ fork05 fork05 fork06 fork_procs -n 1000 fork07 fork07 fork08 fork08 -fork09 fork09 fork10 fork10 fork11 fork_procs -n 100 fork13 fork13 diff --git a/testcases/kernel/syscalls/fork/.gitignore b/testcases/kernel/syscalls/fork/.gitignore index 55e30edb4..6a55df765 100644 --- a/testcases/kernel/syscalls/fork/.gitignore +++ b/testcases/kernel/syscalls/fork/.gitignore @@ -4,7 +4,6 @@ /fork05 /fork07 /fork08 -/fork09 /fork10 /fork12 /fork13 diff --git a/testcases/kernel/syscalls/fork/fork09.c b/testcases/kernel/syscalls/fork/fork09.c deleted file mode 100644 index 32bad89b3..000000000 --- a/testcases/kernel/syscalls/fork/fork09.c +++ /dev/null @@ -1,172 +0,0 @@ -/* - * 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 - * - * NAME - * fork09.c - * - * DESCRIPTION - * Check that child has access to a full set of files. - * - * ALGORITHM - * Parent opens a maximum number of files - * Child closes one and attempts to open another, it should be - * available - * - * USAGE - * fork09 - * - * HISTORY - * 07/2001 Ported by Wayne Boyer - * - * 10/2008 Suzuki K P <suzuki@in.ibm.com> - * Fix maximum number of files open logic. - * - * RESTRICTIONS - * None - */ - -#include <sys/types.h> -#include <sys/wait.h> -#include <sys/stat.h> -#include <fcntl.h> -#include <stdio.h> -#include <errno.h> -#include <unistd.h> /* for _SC_OPEN_MAX */ -#include "test.h" -#include "safe_macros.h" - -char *TCID = "fork09"; -int TST_TOTAL = 1; - -static void setup(void); -static void cleanup(void); - -static char filname[40], childfile[40]; -static int first; -static FILE **fildeses; /* file streams */ -static int mypid, nfiles; - -#define OPEN_MAX (sysconf(_SC_OPEN_MAX)) - -int main(int ac, char **av) -{ - int pid, status, nf; - - int lc; - - tst_parse_opts(ac, av, NULL, NULL); - - setup(); - - fildeses = malloc((OPEN_MAX + 10) * sizeof(FILE *)); - if (fildeses == NULL) - tst_brkm(TBROK, cleanup, "malloc failed"); - - for (lc = 0; TEST_LOOPING(lc); lc++) { - tst_count = 0; - mypid = getpid(); - - tst_resm(TINFO, "OPEN_MAX is %ld", OPEN_MAX); - - /* establish first free file */ - sprintf(filname, "fork09.%d", mypid); - first = SAFE_CREAT(cleanup, filname, 0660); - close(first); - - tst_resm(TINFO, "first file descriptor is %d ", first); - - SAFE_UNLINK(cleanup, filname); - - /* - * now open all the files for the test - */ - for (nfiles = first; nfiles < OPEN_MAX; nfiles++) { - sprintf(filname, "file%d.%d", nfiles, mypid); - fildeses[nfiles] = fopen(filname, "a"); - if (fildeses[nfiles] == NULL) { - /* Did we already reach OPEN_MAX ? */ - if (errno == EMFILE) - break; - tst_brkm(TBROK, cleanup, "Parent: cannot open " - "file %d %s errno = %d", nfiles, - filname, errno); - } -#ifdef DEBUG - tst_resm(TINFO, "filname: %s", filname); -#endif - } - - tst_resm(TINFO, "Parent reporting %d files open", nfiles - 1); - - pid = fork(); - if (pid == -1) - tst_brkm(TBROK, cleanup, "Fork failed"); - - if (pid == 0) { /* child */ - nfiles--; - if (fclose(fildeses[nfiles]) == -1) { - tst_resm(TINFO, "Child could not close file " - "#%d, errno = %d", nfiles, errno); - exit(1); - } else { - sprintf(childfile, "cfile.%d", getpid()); - fildeses[nfiles] = fopen(childfile, "a"); - if (fildeses[nfiles] == NULL) { - tst_resm(TINFO, "Child could not open " - "file %s, errno = %d", - childfile, errno); - exit(1); - } else { - tst_resm(TINFO, "Child opened new " - "file #%d", nfiles); - unlink(childfile); - exit(0); - } - } - } else { /* parent */ - wait(&status); - if (status >> 8 != 0) - tst_resm(TFAIL, "test 1 FAILED"); - else - tst_resm(TPASS, "test 1 PASSED"); - } - - /* clean up things in case we are looping */ - for (nf = first; nf < nfiles; nf++) { - fclose(fildeses[nf]); - sprintf(filname, "file%d.%d", nf, mypid); - unlink(filname); - } - } - - cleanup(); - tst_exit(); -} - -static void setup(void) -{ - tst_sig(FORK, DEF_HANDLER, cleanup); - umask(0); - - TEST_PAUSE; - tst_tmpdir(); -} - -static void cleanup(void) -{ - tst_rmdir(); -}