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

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> +#define FILE_NAME "aiodio_append"
>  #define NUM_CHILDREN 8
> +#define NUM_AIO 16
> +#define AIO_SIZE (64 * 1024)
> +#define FILE_SIZE (64 * 1024)

I guess that we should make these as defaults and allow the number of
children, etc. to be changed via command line parameters later on in an
subsequent patches...

> +static int *run_child;
>  
> -int read_eof(char *filename)
> +static void read_eof(const char *filename, size_t bs)
>  {
>  	int fd;
> -	int i;
>  	int r;
> -	char buf[4096];
> +	char *bufptr;
>  
> -	while ((fd = open(filename, O_RDONLY)) < 0) {
> -		sleep(1);	/* wait for file to be created */
> -	}
> +	while ((fd = open(filename, O_RDONLY, 0666)) < 0)
> +		usleep(100);
>  
> -	for (i = 0; i < 1000000; i++) {
> +	bufptr = SAFE_MEMALIGN(getpagesize(), bs);
> +
> +	tst_res(TINFO, "child %i reading file", getpid());
> +	while (*run_child) {
>  		off_t offset;
>  		char *bufoff;
>  
> -		offset = lseek(fd, SEEK_END, 0);
> -		r = read(fd, buf, 4096);
> -		if (r > 0) {
> -			if ((bufoff = check_zero(buf, r))) {
> -				fprintf(stderr, "non-zero read at offset %p\n",
> -					offset + bufoff);
> -				exit(1);
> +		offset = SAFE_LSEEK(fd, 0, SEEK_END);
> +		do {
> +			r = SAFE_READ(0, fd, bufptr, bs);
> +			if (r > 0) {
> +				bufoff = check_zero(bufptr, r);
> +				if (bufoff) {
> +					tst_res(TINFO, "non-zero read at offset %zu",
> +						offset + (bufoff - bufptr));
> +					free(bufptr);
> +					SAFE_CLOSE(fd);
> +					return;
> +				}
> +				offset += r;
>  			}
> -		}
> +		} while (r > 0);
>  	}
> -	return 0;
> -}
>  
> -#define NUM_AIO 16
> -#define AIO_SIZE 64*1024
> +	free(bufptr);
> +	SAFE_CLOSE(fd);
> +}

My comment for v1 still applies here, there should be only one instance
of this loop in the source tree.

The rest of the changes does look OK.


-- 
Cyril Hrubis
chrubis@suse.cz

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