Re: [LTP] [PATCH v1] Refactoring aiodio_append.c using LTP API

[Petr Vorel <pvorel@suse.cz>]

Hi Andrea,

...
> +static void run(void)
> +{
> +	char *filename = "file";
You decided to remove file as first parameter.
That's fine ($TMPDIR/aiodio.$$/file2 as parameter used $$ to get unique
directory, but new API uses unique directory anyway), but then you should update
runtest/ltp-aiodio.part4 - remove $TMPDIR/aiodio.$$/file2.
nit: I'd just define that name with preprocessor #define.

> +	int filesize = FILE_SIZE;
> +	int num_children = NUM_CHILDREN;
> +	int status;
>  	int i;
> -	char *filename = argv[1];

> -	printf("Starting aio/dio append test...\n");
> +	*run_child = 1;

>  	for (i = 0; i < num_children; i++) {
nit: I'd use directly NUM_CHILDREN
> -		if ((pid[i] = fork()) == 0) {
> -			/* child */
> -			return read_eof(filename);
> -		} else if (pid[i] < 0) {
> -			/* error */
> -			perror("fork error");
> -			break;
> -		} else {
> -			/* Parent */
> -			continue;
> +		if (!SAFE_FORK()) {
> +			read_eof(filename, filesize);
... and here FILE_SIZE
> +			return;
>  		}
>  	}
...

> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.needs_tmpdir = 1,
> +	.forks_child = 1,
> +};
>  #else
> -int main(void)
> +static void run(void)
>  {
> -	tst_brkm(TCONF, NULL, "test requires libaio and it's development packages");
> +	tst_res(TCONF, "test requires libaio and it's development packages");
>  }
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +};
>  #endif

New API has shorter variant when need to quit due missing dependencies:
TST_TEST_TCONF(), e.g.:

#else
	TST_TEST_TCONF("test requires libaio and it's development packages");
#endif

Kind regards,
Petr

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