Message ID | 20230316072231.19157-3-wegao@suse.com |
---|---|
State | Changes Requested |
Headers | show |
Series | Refactor semaphore | expand |
Hi Wei nit: in subject we use space after colon (readability): "semop04:Refactor with new API" => "semop04: Refactor with new API" > Signed-off-by: Wei Gao <wegao@suse.com> > --- > testcases/kernel/syscalls/ipc/semop/semop04.c | 159 ++++++------------ > 1 file changed, 56 insertions(+), 103 deletions(-) > diff --git a/testcases/kernel/syscalls/ipc/semop/semop04.c b/testcases/kernel/syscalls/ipc/semop/semop04.c > index 582624d60..589d9b0b4 100644 > --- a/testcases/kernel/syscalls/ipc/semop/semop04.c > +++ b/testcases/kernel/syscalls/ipc/semop/semop04.c > @@ -1,36 +1,19 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > /* > + * Copyright (c) International Business Machines Corp., 2001 > + * Copyright (C) 2003-2023 Linux Test Project, Inc. > + * Author: 2001 Paul Larson <plars@us.ibm.com> > + * Modified: 2001 Manoj Iyer <manjo@ausin.ibm.com> > * nit: Please remove this line with just star ('*'), when it follows */ > - * 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 > */ > -/* > - * FILE : sem01.c > - * DESCRIPTION : Creates a semaphore and two processes. The processes > - * each go through a loop where they semdown, delay for a > - * random amount of time, and semup, so they will almost > - * always be fighting for control of the semaphore. > - * HISTORY: > - * 01/15/2001 Paul Larson (plars@us.ibm.com) > - * -written > - * 11/09/2001 Manoj Iyer (manjo@ausin.ibm.com) > - * Modified. > - * - Removed compiler warnings. > - * added exit to the end of function main() > +/*\ > + * [Description] > * > + * Creates a semaphore and two processes. The processes > + * each go through a loop where they semdown, delay for a > + * random amount of time, and semup, so they will almost > + * always be fighting for control of the semaphore. > */ > #include <unistd.h> > @@ -41,124 +24,94 @@ > #include <sys/wait.h> nit: I'd remove this (we no longer use waitpid()). Also I'd remove <errno.h> (IMHO no errno is used). > #include <sys/ipc.h> > #include "lapi/sem.h" > +#include "tst_test.h" > +#include "tst_safe_sysv_ipc.h" > -int verbose = 0; > -int loops = 100; > -int errors = 0; > +static int loops = 100; > +static char *opt_loops_str; I'm not sure if this worth to have opt for loops. I'd just define it: #define LOOPS 1000 and not allow to overwrite (even with 1000 it's fast). > -int semup(int semid) > +static void semup(int semid) > { > struct sembuf semops; > + > semops.sem_num = 0; > semops.sem_op = 1; > semops.sem_flg = SEM_UNDO; > - if (semop(semid, &semops, 1) == -1) { > - perror("semup"); > - errors++; > - return 1; > - } > - return 0; > + > + SAFE_SEMOP(semid, &semops, 1); > } > -int semdown(int semid) > +static void semdown(int semid) > { > struct sembuf semops; > + > semops.sem_num = 0; > semops.sem_op = -1; > semops.sem_flg = SEM_UNDO; > - if (semop(semid, &semops, 1) == -1) { > - perror("semdown"); > - errors++; > - return 1; > - } > - return 0; > + > + SAFE_SEMOP(semid, &semops, 1); > } > -void delayloop() > +static void delayloop(void) > { > int delay; > + > delay = 1 + ((100.0 * rand()) / RAND_MAX); > - if (verbose) > - printf("in delay function for %d microseconds\n", delay); > usleep(delay); > } > void mainloop(int semid) This should also be static. > { > int i; ... > + SAFE_SEMCTL(semid, 0, SETVAL, semunion); > + > + pid = SAFE_FORK(); > + > if (pid) { > /* parent */ Please remove this obvious comment. > srand(pid); > mainloop(semid); ... > + tst_reap_children(); > + SAFE_SEMCTL(semid, 0, IPC_RMID, semunion); > + tst_res(TPASS, "Semaphore up/down check success"); > } else { > /* child */ And this one. > mainloop(semid); > } > - exit(0); > } > + > +static void setup(void) > +{ > + if (opt_loops_str) > + loops = SAFE_STRTOL(opt_loops_str, 1, INT_MAX); > +} This is to be removed. > + > +static struct tst_test test = { > + .test_all = run, > + .setup = setup, > + .forks_child = 1, > + .options = (struct tst_option[]) { > + {"l:", &opt_loops_str, "Internal loops for semup/down"}, > + {} This is going to be removed. > + }, > + .needs_root = 1, Do we really need root? I guess no. Kind regards, Petr > +};
diff --git a/testcases/kernel/syscalls/ipc/semop/semop04.c b/testcases/kernel/syscalls/ipc/semop/semop04.c index 582624d60..589d9b0b4 100644 --- a/testcases/kernel/syscalls/ipc/semop/semop04.c +++ b/testcases/kernel/syscalls/ipc/semop/semop04.c @@ -1,36 +1,19 @@ +// SPDX-License-Identifier: GPL-2.0-or-later /* + * Copyright (c) International Business Machines Corp., 2001 + * Copyright (C) 2003-2023 Linux Test Project, Inc. + * Author: 2001 Paul Larson <plars@us.ibm.com> + * Modified: 2001 Manoj Iyer <manjo@ausin.ibm.com> * - * 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 */ -/* - * FILE : sem01.c - * DESCRIPTION : Creates a semaphore and two processes. The processes - * each go through a loop where they semdown, delay for a - * random amount of time, and semup, so they will almost - * always be fighting for control of the semaphore. - * HISTORY: - * 01/15/2001 Paul Larson (plars@us.ibm.com) - * -written - * 11/09/2001 Manoj Iyer (manjo@ausin.ibm.com) - * Modified. - * - Removed compiler warnings. - * added exit to the end of function main() +/*\ + * [Description] * + * Creates a semaphore and two processes. The processes + * each go through a loop where they semdown, delay for a + * random amount of time, and semup, so they will almost + * always be fighting for control of the semaphore. */ #include <unistd.h> @@ -41,124 +24,94 @@ #include <sys/wait.h> #include <sys/ipc.h> #include "lapi/sem.h" +#include "tst_test.h" +#include "tst_safe_sysv_ipc.h" -int verbose = 0; -int loops = 100; -int errors = 0; +static int loops = 100; +static char *opt_loops_str; -int semup(int semid) +static void semup(int semid) { struct sembuf semops; + semops.sem_num = 0; semops.sem_op = 1; semops.sem_flg = SEM_UNDO; - if (semop(semid, &semops, 1) == -1) { - perror("semup"); - errors++; - return 1; - } - return 0; + + SAFE_SEMOP(semid, &semops, 1); } -int semdown(int semid) +static void semdown(int semid) { struct sembuf semops; + semops.sem_num = 0; semops.sem_op = -1; semops.sem_flg = SEM_UNDO; - if (semop(semid, &semops, 1) == -1) { - perror("semdown"); - errors++; - return 1; - } - return 0; + + SAFE_SEMOP(semid, &semops, 1); } -void delayloop() +static void delayloop(void) { int delay; + delay = 1 + ((100.0 * rand()) / RAND_MAX); - if (verbose) - printf("in delay function for %d microseconds\n", delay); usleep(delay); } void mainloop(int semid) { int i; + for (i = 0; i < loops; i++) { - if (semdown(semid)) { - printf("semdown failed\n"); - } - if (verbose) - printf("sem is down\n"); + semdown(semid); delayloop(); - if (semup(semid)) { - printf("semup failed\n"); - } - if (verbose) - printf("sem is up\n"); + semup(semid); } } -int main(int argc, char *argv[]) +static void run(void) { - int semid, opt; + int semid; union semun semunion; - extern char *optarg; pid_t pid; - int chstat; - - while ((opt = getopt(argc, argv, "l:vh")) != EOF) { - switch ((char)opt) { - case 'l': - loops = atoi(optarg); - break; - case 'v': - verbose = 1; - break; - case 'h': - default: - printf("Usage: -l loops [-v]\n"); - exit(1); - } - } /* set up the semaphore */ - if ((semid = semget((key_t) 9142, 1, 0666 | IPC_CREAT)) < 0) { - printf("error in semget()\n"); - exit(-1); - } + semid = SAFE_SEMGET((key_t) 9142, 1, 0666 | IPC_CREAT); + semunion.val = 1; - if (semctl(semid, 0, SETVAL, semunion) == -1) { - printf("error in semctl\n"); - } - if ((pid = fork()) < 0) { - printf("fork error\n"); - exit(-1); - } + SAFE_SEMCTL(semid, 0, SETVAL, semunion); + + pid = SAFE_FORK(); + if (pid) { /* parent */ srand(pid); mainloop(semid); - waitpid(pid, &chstat, 0); - if (!WIFEXITED(chstat)) { - printf("child exited with status\n"); - exit(-1); - } - if (semctl(semid, 0, IPC_RMID, semunion) == -1) { - printf("error in semctl\n"); - } - if (errors) { - printf("FAIL: there were %d errors\n", errors); - } else { - printf("PASS: error count is 0\n"); - } - exit(errors); + tst_reap_children(); + SAFE_SEMCTL(semid, 0, IPC_RMID, semunion); + tst_res(TPASS, "Semaphore up/down check success"); } else { /* child */ mainloop(semid); } - exit(0); } + +static void setup(void) +{ + if (opt_loops_str) + loops = SAFE_STRTOL(opt_loops_str, 1, INT_MAX); +} + +static struct tst_test test = { + .test_all = run, + .setup = setup, + .forks_child = 1, + .options = (struct tst_option[]) { + {"l:", &opt_loops_str, "Internal loops for semup/down"}, + {} + }, + .needs_root = 1, +};
Signed-off-by: Wei Gao <wegao@suse.com> --- testcases/kernel/syscalls/ipc/semop/semop04.c | 159 ++++++------------ 1 file changed, 56 insertions(+), 103 deletions(-)