Re: [LTP] [PATCH v4] Refactor aiocp using new LTP API

[Petr Vorel <pvorel@suse.cz>]

Hi Andrea,

...
> +	io_destroy(myctx);
> +	SAFE_CLOSE(srcfd);
> +	SAFE_CLOSE(dstfd);
> +
> +	/* check if file has been copied correctly */
> +	tst_res(TINFO, "Comparing %s with %s", srcname, dstname);
> +
> +	SAFE_STAT(dstname, &st);
> +	if (st.st_size != filesize) {
> +		tst_res(TFAIL, "Expected destination file size %lld but it's %ld", filesize, st.st_size);
> +		/* no need to compare files */
> +		return;
>  	}
> +	srcfd = SAFE_OPEN(srcname, srcflags | O_RDONLY, 0666);
> +	dstfd = SAFE_OPEN(dstname, srcflags | O_RDONLY, 0666);
> +
> +	reads = howmany(filesize, buffsize);
> +
> +	for (i = 0; i < reads; i++) {
> +		r = SAFE_READ(0, srcfd, srcbuff, buffsize);
> +		SAFE_READ(0, dstfd, dstbuff, buffsize);
> +		if (memcmp(srcbuff, dstbuff, r)) {
> +			fail = 1;
> +			break;
>  		}

> +	SAFE_CLOSE(srcfd);
> +	SAFE_CLOSE(dstfd);
Shouldn't this be moved to the cleanup function? Because if SAFE_STAT() fails, file
stays open (also, if you add the cleanup function, than tst_res(TFAIL) can be in
for loop, followed by return - a bit simpler code).

Otherwise LGTM.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

> +	if (fail)
> +		tst_res(TFAIL, "Files are not identical");
> +	else
> +		tst_res(TPASS, "Files are identical");
> +}

> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.needs_tmpdir = 1,
> +	.options = (struct tst_option[]) {
> +		{"b:", &str_aio_blksize, "Size of writing blocks (default 1K)"},
> +		{"s:", &str_filesize, "Size of file (default 10M)"},
> +		{"n:", &str_aionum, "Number of Async IO blocks (default 16)"},
> +		{"f:", &str_oflag, "Open flag: SYNC | DIRECT (default O_CREAT only)"},
> +		{},
> +	},
> +};
>  #else
> -int main(void)
> -{
> -	fprintf(stderr, "test requires libaio and it's development packages\n");
> -	return TCONF;
> -}
> +TST_TEST_TCONF("test requires libaio and its development packages");
>  #endif

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