Re: [LTP] [v7,2/2] semop04: Refactor with new API

[Andrea Cervesato via ltp <ltp@lists.linux.it>]

Hi!

On 8/29/23 13:18, Wei Gao via ltp wrote:
> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>   testcases/kernel/syscalls/ipc/semop/semop04.c | 158 +++++-------------
>   1 file changed, 42 insertions(+), 116 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/ipc/semop/semop04.c b/testcases/kernel/syscalls/ipc/semop/semop04.c
> index 582624d60..96f4b8fb8 100644
> --- a/testcases/kernel/syscalls/ipc/semop/semop04.c
> +++ b/testcases/kernel/syscalls/ipc/semop/semop04.c
> @@ -1,164 +1,90 @@
> +// 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) 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>
>    */
>   
> -/*
> - *  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>
>   #include <stdlib.h>
>   #include <stdio.h>
> -#include <errno.h>
>   #include <sys/types.h>
> -#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;
> +#define LOOPS 1000
>   
> -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;
> -}
>   
> -void delayloop()
> -{
> -	int delay;
> -	delay = 1 + ((100.0 * rand()) / RAND_MAX);
> -	if (verbose)
> -		printf("in delay function for %d microseconds\n", delay);
> -	usleep(delay);
> +	SAFE_SEMOP(semid, &semops, 1);
>   }
>   
> -void mainloop(int semid)
> +static 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");
> -		delayloop();
> -		if (semup(semid)) {
> -			printf("semup failed\n");
> -		}
> -		if (verbose)
> -			printf("sem is up\n");
> +
> +	for (i = 0; i < LOOPS; i++) {
> +		semdown(semid);
> +		usleep(1 + ((100.0 * rand()) / RAND_MAX));
> +		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);
I'm wondering if this is an error from the previous test as well. We are 
using mainloop() function that is using rand() inside both child and 
parent. So srand() should be called before parent and child, in order to 
have random seed initialized for both of them. I would also use 
srand(time(0)).
>   		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();
> +		TST_EXP_POSITIVE(semctl(semid, 0, IPC_RMID, semunion));
>   	} else {
> -		/* child */
>   		mainloop(semid);
>   	}
> -	exit(0);
>   }
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.forks_child = 1,
> +};

Thank you,
Andrea Cervesato


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp