Re: [PATCH v3 3/7] selftests/powerpc: Add generic read/write file util

[Michael Ellerman <mpe@ellerman.id.au>]

Benjamin Gray <bgray@linux.ibm.com> writes:
> File read/write is reimplemented in about 5 different ways in the
> various PowerPC selftests. This indicates it should be a common util.
>
> Add a common read_file / write_file implementation and convert users
> to it where (easily) possible.
>
> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
> ---
>  tools/testing/selftests/powerpc/dscr/dscr.h   |  36 ++----
>  .../selftests/powerpc/dscr/dscr_sysfs_test.c  |  19 +--
>  .../testing/selftests/powerpc/include/utils.h |   2 +
>  .../selftests/powerpc/nx-gzip/gzfht_test.c    |  49 +++-----
>  tools/testing/selftests/powerpc/pmu/lib.c     |  27 +----
>  .../selftests/powerpc/ptrace/core-pkey.c      |  30 ++---
>  tools/testing/selftests/powerpc/utils.c       | 108 ++++++++++--------
>  7 files changed, 107 insertions(+), 164 deletions(-)
>
> diff --git a/tools/testing/selftests/powerpc/dscr/dscr.h b/tools/testing/selftests/powerpc/dscr/dscr.h
> index b703714e7d98..9a69d473ffdf 100644
> --- a/tools/testing/selftests/powerpc/dscr/dscr.h
> +++ b/tools/testing/selftests/powerpc/dscr/dscr.h
> @@ -64,48 +64,30 @@ inline void set_dscr_usr(unsigned long val)
>  /* Default DSCR access */
>  unsigned long get_default_dscr(void)
>  {
> -	int fd = -1, ret;
> -	char buf[16];
> +	int err;
> +	char buf[16] = {0};
>  	unsigned long val;
>  
> -	if (fd == -1) {
> -		fd = open(DSCR_DEFAULT, O_RDONLY);
> -		if (fd == -1) {
> -			perror("open() failed");
> -			exit(1);
> -		}
> -	}
> -	memset(buf, 0, sizeof(buf));
> -	lseek(fd, 0, SEEK_SET);
> -	ret = read(fd, buf, sizeof(buf));
> -	if (ret == -1) {
> -		perror("read() failed");
> +	if ((err = read_file(DSCR_DEFAULT, buf, sizeof(buf) - 1, NULL))) {
> +		fprintf(stderr, "get_default_dscr() read failed: %s\n", strerror(err));

I don't particularly like doing the assignment to err in the if.

And checkpatch flags it as an error, which means even if we did like it
we'd be setting ourselves up for a stream of fixup patches :)

So please just do the assignment and the if separately.

cheers