[LTP] [PATCH v2] dio_truncate.c test refactory with LTP API

[LTP] [PATCH v2] dio_truncate.c test refactory with LTP API

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

---
 testcases/kernel/io/ltp-aiodio/dio_truncate.c | 206 +++++++-----------
 1 file changed, 79 insertions(+), 127 deletions(-)

diff --git a/testcases/kernel/io/ltp-aiodio/dio_truncate.c b/testcases/kernel/io/ltp-aiodio/dio_truncate.c
index 27cf01525..9c8da539e 100644
--- a/testcases/kernel/io/ltp-aiodio/dio_truncate.c
+++ b/testcases/kernel/io/ltp-aiodio/dio_truncate.c
@@ -1,7 +1,7 @@
-
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * Copyright (c) 2004 Daniel McNeil <daniel@osdl.org>
- *               2004 Open Source Development Lab
+ * Copyright (c) 2021 Andrea Cervesato <andrea.cervesato@suse.com>
+ *			   2021 SUSE Enterprise Linux
  *   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
@@ -19,159 +19,111 @@
  * Module: .c
  */
 
-/*
- * Change History:
- *
- * 2/2004  Marty Ridgeway (mridge@us.ibm.com) Changes to adapt to LTP
- *
- */
 #define _GNU_SOURCE
 
 #include <stdlib.h>
+#include <stdio.h>
+#include <sys/stat.h>
 #include <sys/types.h>
-#include <signal.h>
-#include <errno.h>
 #include <fcntl.h>
-#include <stdio.h>
-#include <unistd.h>
-#include <memory.h>
-#include <string.h>
-#include <limits.h>
-
-#include "test.h"
+#include "tst_test.h"
 
-#define NUM_CHILDREN 8
+static char *FILENAME = "file.bin";
+static long FILESIZE = 64 * 1024;
+static int STARTING_CHARS = 10;
 
-char *check_zero(unsigned char *buf, int size)
-{
-	unsigned char *p;
-
-	p = buf;
-
-	while (size > 0) {
-		if (*buf != 0) {
-			fprintf(stderr,
-				"non zero buffer at buf[%d] => 0x%02x,%02x,%02x,%02x\n",
-				buf - p, (unsigned int)buf[0],
-				size > 1 ? (unsigned int)buf[1] : 0,
-				size > 2 ? (unsigned int)buf[2] : 0,
-				size > 3 ? (unsigned int)buf[3] : 0);
-			fprintf(stderr, "buf %p, p %p\n", buf, p);
-			return buf;
-		}
-		buf++;
-		size--;
-	}
-	return 0;		/* all zeros */
-}
-
-int dio_read(char *filename)
+int dio_append(const char *path, char pattern, size_t bs, size_t bcount)
 {
 	int fd;
-	int r;
-	void *bufptr = NULL;
 
-	TEST(posix_memalign(&bufptr, 4096, 64 * 1024));
-	if (TEST_RETURN) {
-		tst_resm(TBROK | TRERRNO, "cannot malloc aligned memory");
+	fd = SAFE_OPEN(path, O_CREAT|O_WRONLY|O_DIRECT, 0666);
+
+	if (tst_fill_fd(fd, pattern, bs, bcount)) {
+		SAFE_CLOSE(fd);
 		return -1;
 	}
 
-	while ((fd = open(filename, O_DIRECT | O_RDONLY)) < 0) {
-	}
-	fprintf(stderr, "dio_truncate: child reading file\n");
-	while (1) {
-		off_t offset;
-		char *bufoff;
-
-		/* read the file, checking for zeros */
-		offset = lseek(fd, SEEK_SET, 0);
-		do {
-			r = read(fd, bufptr, 64 * 1024);
-			if (r > 0) {
-				if ((bufoff = check_zero(bufptr, r))) {
-					fprintf(stderr,
-						"non-zero read at offset %p\n",
-						offset + bufoff);
-					exit(1);
-				}
-				offset += r;
-			}
-		} while (r > 0);
-	}
+	SAFE_CLOSE(fd);
+
 	return 0;
 }
 
-void dio_append(char *filename, int fill)
+int dio_get_zeros(const char *path, size_t bs)
 {
-	int fd;
+	int i = 0;
+	int fd = 0;
+	int zeros = 0;
 	void *bufptr = NULL;
-	int i;
-	int w;
 
-	fd = open(filename, O_DIRECT | O_WRONLY | O_CREAT, 0666);
+	fd = SAFE_OPEN(path, O_RDONLY|O_DIRECT, 0666);
 
-	if (fd < 0) {
-		perror("cannot create file");
-		return;
-	}
+	bufptr = SAFE_MALLOC(bs * sizeof(void));
+	SAFE_READ(0, fd, bufptr, bs);
 
-	TEST(posix_memalign(&bufptr, 4096, 64 * 1024));
-	if (TEST_RETURN) {
-		tst_resm(TBROK | TRERRNO, "cannot malloc aligned memory");
-		close(fd);
-		return;
+	for (i = 0; i < (int)bs; i++) {
+		if (*(char *)bufptr == 0)
+			zeros++;
+		bufptr++;
 	}
 
-	memset(bufptr, fill, 64 * 1024);
+	SAFE_CLOSE(fd);
 
-	for (i = 0; i < 1000; i++) {
-		if ((w = write(fd, bufptr, 64 * 1024)) != 64 * 1024) {
-			fprintf(stderr, "write %d returned %d\n", i, w);
-		}
-	}
-	close(fd);
+	return zeros;
 }
 
-int main(void)
+off_t getsize(const char *filename)
 {
-	char filename[PATH_MAX];
-	int pid[NUM_CHILDREN];
-	int num_children = 1;
-	int i;
-
-	snprintf(filename, sizeof(filename), "%s/aiodio/file",
-		 getenv("TMP") ? getenv("TMP") : "/tmp");
-
-	for (i = 0; i < num_children; i++) {
-		if ((pid[i] = fork()) == 0) {
-			/* child */
-			return dio_read(filename);
-		} else if (pid[i] < 0) {
-			/* error */
-			perror("fork error");
-			break;
-		} else {
-			/* Parent */
-			continue;
-		}
-	}
+	struct stat st;
 
-	/*
-	 * Parent creates a zero file using DIO.
-	 * Truncates it to zero
-	 * Create another file with '0xaa'
-	 */
-	for (i = 0; i < 100; i++) {
-		dio_append(filename, 0);
-		truncate(filename, 0);
-		dio_append("junkfile", 0xaa);
-		truncate("junkfile", 0);
-	}
+	if (SAFE_STAT(filename, &st) == 0)
+		return st.st_size;
+
+	return -1;
+}
+
+static void run(void)
+{
+	int charnum = 0;
+	long empty_ch = FILESIZE - STARTING_CHARS;
+
+	tst_res(TINFO, "Create %s filled with %d chars", FILENAME, STARTING_CHARS);
+	dio_append(FILENAME, 'a', STARTING_CHARS, 1);
 
-	for (i = 0; i < num_children; i++) {
-		kill(pid[i], SIGTERM);
+	/* Truncate to a bigger file and check if it's filled with empty chars */
+	tst_res(TINFO, "Truncate to %ld", FILESIZE);
+	TST_EXP_POSITIVE(truncate(FILENAME, FILESIZE), "truncate(%s, %lu)", FILENAME, FILESIZE);
+	if (!TST_PASS)
+		return;
+
+	TEST(getsize(FILENAME));
+
+	if (TST_RET == FILESIZE) {
+		tst_res(TPASS, "Truncated file has %ld length", TST_RET);
+
+		charnum = dio_get_zeros(FILENAME, FILESIZE);
+
+		if (charnum == empty_ch)
+			tst_res(TPASS, "Truncated file provides %ld empty chars at the end", empty_ch);
+		else
+			tst_res(TFAIL, "Truncated file isn't filled with %i chars", charnum);
+	} else {
+		tst_res(TFAIL, "Truncated file doesn't have %ld length but %ld", FILESIZE, TST_RET);
 	}
 
-	return 0;
+	/* Truncate to zero: file must be empty */
+	tst_res(TINFO, "Truncate to zero");
+	TST_EXP_POSITIVE(truncate(FILENAME, 0), "truncate(%s, 0)", FILENAME);
+	if (!TST_PASS)
+		return;
+
+	TEST(getsize(FILENAME));
+	if (TST_RET == 0)
+		tst_res(TPASS, "Truncated file has zero length");
+	else
+		tst_res(TFAIL, "Truncated file doesn't have zero length");
 }
+
+static struct tst_test test = {
+	.needs_tmpdir = 1,
+	.test_all = run,
+};
-- 
2.33.1


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

Re: [LTP] [PATCH v2] dio_truncate.c test refactory with LTP API

[Andrea Cervesato via ltp]

[-- Attachment #1.1: Type: text/plain, Size: 6987 bytes --]

Hi all,

please ignore this patch since it has been sent by accident: it's the 
v1. I sent the correct v2 patch in the next email.

Andrea

On 11/15/21 14:34, Andrea Cervesato wrote:
> From: Andrea Cervesato<andrea.cervesato@suse.com>
>
> ---
>   testcases/kernel/io/ltp-aiodio/dio_truncate.c | 206 +++++++-----------
>   1 file changed, 79 insertions(+), 127 deletions(-)
>
> diff --git a/testcases/kernel/io/ltp-aiodio/dio_truncate.c b/testcases/kernel/io/ltp-aiodio/dio_truncate.c
> index 27cf01525..9c8da539e 100644
> --- a/testcases/kernel/io/ltp-aiodio/dio_truncate.c
> +++ b/testcases/kernel/io/ltp-aiodio/dio_truncate.c
> @@ -1,7 +1,7 @@
> -
> +// SPDX-License-Identifier: GPL-2.0-or-later
>   /*
> - * Copyright (c) 2004 Daniel McNeil<daniel@osdl.org>
> - *               2004 Open Source Development Lab
> + * Copyright (c) 2021 Andrea Cervesato<andrea.cervesato@suse.com>
> + *			   2021 SUSE Enterprise Linux
>    *   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
> @@ -19,159 +19,111 @@
>    * Module: .c
>    */
>   
> -/*
> - * Change History:
> - *
> - * 2/2004  Marty Ridgeway (mridge@us.ibm.com) Changes to adapt to LTP
> - *
> - */
>   #define _GNU_SOURCE
>   
>   #include <stdlib.h>
> +#include <stdio.h>
> +#include <sys/stat.h>
>   #include <sys/types.h>
> -#include <signal.h>
> -#include <errno.h>
>   #include <fcntl.h>
> -#include <stdio.h>
> -#include <unistd.h>
> -#include <memory.h>
> -#include <string.h>
> -#include <limits.h>
> -
> -#include "test.h"
> +#include "tst_test.h"
>   
> -#define NUM_CHILDREN 8
> +static char *FILENAME = "file.bin";
> +static long FILESIZE = 64 * 1024;
> +static int STARTING_CHARS = 10;
>   
> -char *check_zero(unsigned char *buf, int size)
> -{
> -	unsigned char *p;
> -
> -	p = buf;
> -
> -	while (size > 0) {
> -		if (*buf != 0) {
> -			fprintf(stderr,
> -				"non zero buffer at buf[%d] => 0x%02x,%02x,%02x,%02x\n",
> -				buf - p, (unsigned int)buf[0],
> -				size > 1 ? (unsigned int)buf[1] : 0,
> -				size > 2 ? (unsigned int)buf[2] : 0,
> -				size > 3 ? (unsigned int)buf[3] : 0);
> -			fprintf(stderr, "buf %p, p %p\n", buf, p);
> -			return buf;
> -		}
> -		buf++;
> -		size--;
> -	}
> -	return 0;		/* all zeros */
> -}
> -
> -int dio_read(char *filename)
> +int dio_append(const char *path, char pattern, size_t bs, size_t bcount)
>   {
>   	int fd;
> -	int r;
> -	void *bufptr = NULL;
>   
> -	TEST(posix_memalign(&bufptr, 4096, 64 * 1024));
> -	if (TEST_RETURN) {
> -		tst_resm(TBROK | TRERRNO, "cannot malloc aligned memory");
> +	fd = SAFE_OPEN(path, O_CREAT|O_WRONLY|O_DIRECT, 0666);
> +
> +	if (tst_fill_fd(fd, pattern, bs, bcount)) {
> +		SAFE_CLOSE(fd);
>   		return -1;
>   	}
>   
> -	while ((fd = open(filename, O_DIRECT | O_RDONLY)) < 0) {
> -	}
> -	fprintf(stderr, "dio_truncate: child reading file\n");
> -	while (1) {
> -		off_t offset;
> -		char *bufoff;
> -
> -		/* read the file, checking for zeros */
> -		offset = lseek(fd, SEEK_SET, 0);
> -		do {
> -			r = read(fd, bufptr, 64 * 1024);
> -			if (r > 0) {
> -				if ((bufoff = check_zero(bufptr, r))) {
> -					fprintf(stderr,
> -						"non-zero read at offset %p\n",
> -						offset + bufoff);
> -					exit(1);
> -				}
> -				offset += r;
> -			}
> -		} while (r > 0);
> -	}
> +	SAFE_CLOSE(fd);
> +
>   	return 0;
>   }
>   
> -void dio_append(char *filename, int fill)
> +int dio_get_zeros(const char *path, size_t bs)
>   {
> -	int fd;
> +	int i = 0;
> +	int fd = 0;
> +	int zeros = 0;
>   	void *bufptr = NULL;
> -	int i;
> -	int w;
>   
> -	fd = open(filename, O_DIRECT | O_WRONLY | O_CREAT, 0666);
> +	fd = SAFE_OPEN(path, O_RDONLY|O_DIRECT, 0666);
>   
> -	if (fd < 0) {
> -		perror("cannot create file");
> -		return;
> -	}
> +	bufptr = SAFE_MALLOC(bs * sizeof(void));
> +	SAFE_READ(0, fd, bufptr, bs);
>   
> -	TEST(posix_memalign(&bufptr, 4096, 64 * 1024));
> -	if (TEST_RETURN) {
> -		tst_resm(TBROK | TRERRNO, "cannot malloc aligned memory");
> -		close(fd);
> -		return;
> +	for (i = 0; i < (int)bs; i++) {
> +		if (*(char *)bufptr == 0)
> +			zeros++;
> +		bufptr++;
>   	}
>   
> -	memset(bufptr, fill, 64 * 1024);
> +	SAFE_CLOSE(fd);
>   
> -	for (i = 0; i < 1000; i++) {
> -		if ((w = write(fd, bufptr, 64 * 1024)) != 64 * 1024) {
> -			fprintf(stderr, "write %d returned %d\n", i, w);
> -		}
> -	}
> -	close(fd);
> +	return zeros;
>   }
>   
> -int main(void)
> +off_t getsize(const char *filename)
>   {
> -	char filename[PATH_MAX];
> -	int pid[NUM_CHILDREN];
> -	int num_children = 1;
> -	int i;
> -
> -	snprintf(filename, sizeof(filename), "%s/aiodio/file",
> -		 getenv("TMP") ? getenv("TMP") : "/tmp");
> -
> -	for (i = 0; i < num_children; i++) {
> -		if ((pid[i] = fork()) == 0) {
> -			/* child */
> -			return dio_read(filename);
> -		} else if (pid[i] < 0) {
> -			/* error */
> -			perror("fork error");
> -			break;
> -		} else {
> -			/* Parent */
> -			continue;
> -		}
> -	}
> +	struct stat st;
>   
> -	/*
> -	 * Parent creates a zero file using DIO.
> -	 * Truncates it to zero
> -	 * Create another file with '0xaa'
> -	 */
> -	for (i = 0; i < 100; i++) {
> -		dio_append(filename, 0);
> -		truncate(filename, 0);
> -		dio_append("junkfile", 0xaa);
> -		truncate("junkfile", 0);
> -	}
> +	if (SAFE_STAT(filename, &st) == 0)
> +		return st.st_size;
> +
> +	return -1;
> +}
> +
> +static void run(void)
> +{
> +	int charnum = 0;
> +	long empty_ch = FILESIZE - STARTING_CHARS;
> +
> +	tst_res(TINFO, "Create %s filled with %d chars", FILENAME, STARTING_CHARS);
> +	dio_append(FILENAME, 'a', STARTING_CHARS, 1);
>   
> -	for (i = 0; i < num_children; i++) {
> -		kill(pid[i], SIGTERM);
> +	/* Truncate to a bigger file and check if it's filled with empty chars */
> +	tst_res(TINFO, "Truncate to %ld", FILESIZE);
> +	TST_EXP_POSITIVE(truncate(FILENAME, FILESIZE), "truncate(%s, %lu)", FILENAME, FILESIZE);
> +	if (!TST_PASS)
> +		return;
> +
> +	TEST(getsize(FILENAME));
> +
> +	if (TST_RET == FILESIZE) {
> +		tst_res(TPASS, "Truncated file has %ld length", TST_RET);
> +
> +		charnum = dio_get_zeros(FILENAME, FILESIZE);
> +
> +		if (charnum == empty_ch)
> +			tst_res(TPASS, "Truncated file provides %ld empty chars at the end", empty_ch);
> +		else
> +			tst_res(TFAIL, "Truncated file isn't filled with %i chars", charnum);
> +	} else {
> +		tst_res(TFAIL, "Truncated file doesn't have %ld length but %ld", FILESIZE, TST_RET);
>   	}
>   
> -	return 0;
> +	/* Truncate to zero: file must be empty */
> +	tst_res(TINFO, "Truncate to zero");
> +	TST_EXP_POSITIVE(truncate(FILENAME, 0), "truncate(%s, 0)", FILENAME);
> +	if (!TST_PASS)
> +		return;
> +
> +	TEST(getsize(FILENAME));
> +	if (TST_RET == 0)
> +		tst_res(TPASS, "Truncated file has zero length");
> +	else
> +		tst_res(TFAIL, "Truncated file doesn't have zero length");
>   }
> +
> +static struct tst_test test = {
> +	.needs_tmpdir = 1,
> +	.test_all = run,
> +};

[-- Attachment #1.2: Type: text/html, Size: 7620 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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

Re: [LTP] [PATCH v2] dio_truncate.c test refactory with LTP API

[Cyril Hrubis]

Hi!
> +	int fd = 0;
> +	size_t i = 0;
> +	char *buf = NULL;

It's pointless to initalize variables that are not read back before they
are set.

> -int dio_read(char *filename)
> -{
> -	int fd;
> -	int r;
> -	void *bufptr = NULL;
> +	fd = SAFE_OPEN(path, O_CREAT|O_WRONLY|O_DIRECT, 0666);
> -	TEST(posix_memalign(&bufptr, 4096, 64 * 1024));
> -	if (TEST_RETURN) {
> -		tst_resm(TBROK | TRERRNO, "cannot malloc aligned memory");
> +	buf = (char *)SAFE_MEMALIGN(getpagesize(), bs);
                ^
		Useless cast

> +	if (buf == 0) {

The most common way to check for NULL pointers in LKML C is just:

	if (!buf) {
		...
	}

> +		SAFE_CLOSE(fd);
>  		return -1;
>  	}
>  
> -	while ((fd = open(filename, O_DIRECT | O_RDONLY)) < 0) {
> -	}
> -	fprintf(stderr, "dio_truncate: child reading file\n");
> -	while (1) {
> -		off_t offset;
> -		char *bufoff;
> -
> -		/* read the file, checking for zeros */
> -		offset = lseek(fd, SEEK_SET, 0);
> -		do {
> -			r = read(fd, bufptr, 64 * 1024);
> -			if (r > 0) {
> -				if ((bufoff = check_zero(bufptr, r))) {
> -					fprintf(stderr,
> -						"non-zero read at offset %p\n",
> -						offset + bufoff);
> -					exit(1);
> -				}
> -				offset += r;
> -			}
> -		} while (r > 0);
> +	for (i = 0; i < bs; i++)
> +		buf[i] = pattern;

Why not just memset(buf, pattern, bs) ?

> +	for (i = 0; i < bcount; i++) {
> +		if (write(fd, buf, bs) != (ssize_t)bs) {
> +			free(buf);

			SAFE_CLOSE(fd) here?

> +			return -1;
> +		}
>  	}
> +
> +	SAFE_CLOSE(fd);
> +
>  	return 0;
>  }
>  
> -void dio_append(char *filename, int fill)
> +int dio_get_zeros(const char *path, size_t bs)

Should be static.

>  {
> -	int fd;
> +	int i = 0;
> +	int fd = 0;
> +	int zeros = 0;
>  	void *bufptr = NULL;
> -	int i;
> -	int w;
>  
> -	fd = open(filename, O_DIRECT | O_WRONLY | O_CREAT, 0666);
> +	fd = SAFE_OPEN(path, O_RDONLY|O_DIRECT, 0666);
                                    ^
				    We usually prefer spaces between bit
				    or as it's easier to read

> -	if (fd < 0) {
> -		perror("cannot create file");
> -		return;
> -	}
> +	bufptr = SAFE_MALLOC(bs * sizeof(void));
                                  ^
				  Is sizeof(void) even defined?!

I guess that in this case it was supposed to be byte array so
sizeof(char) but sizeof(char) is 1 by definition.

> +	SAFE_READ(0, fd, bufptr, bs);

If you pass 0 as the first argument of the SAFE_READ() it can return
less than the requested amount of data.

> -	TEST(posix_memalign(&bufptr, 4096, 64 * 1024));
> -	if (TEST_RETURN) {
> -		tst_resm(TBROK | TRERRNO, "cannot malloc aligned memory");
> -		close(fd);
> -		return;
> +	for (i = 0; i < (int)bs; i++) {
> +		if (*(char *)bufptr == 0)

Can we just define the bufptr as char * instead and use bufptr[i] here?

> +			zeros++;
> +		bufptr++;
>  	}
>  
> -	memset(bufptr, fill, 64 * 1024);
> +	SAFE_CLOSE(fd);
>  
> -	for (i = 0; i < 1000; i++) {
> -		if ((w = write(fd, bufptr, 64 * 1024)) != 64 * 1024) {
> -			fprintf(stderr, "write %d returned %d\n", i, w);
> -		}
> -	}
> -	close(fd);
> +	return zeros;
>  }
>  
> -int main(void)
> +off_t getsize(const char *filename)

Here as well should be static.

>  {
> -	char filename[PATH_MAX];
> -	int pid[NUM_CHILDREN];
> -	int num_children = 1;
> -	int i;
> -
> -	snprintf(filename, sizeof(filename), "%s/aiodio/file",
> -		 getenv("TMP") ? getenv("TMP") : "/tmp");
> -
> -	for (i = 0; i < num_children; i++) {
> -		if ((pid[i] = fork()) == 0) {
> -			/* child */
> -			return dio_read(filename);
> -		} else if (pid[i] < 0) {
> -			/* error */
> -			perror("fork error");
> -			break;
> -		} else {
> -			/* Parent */
> -			continue;
> -		}
> -	}
> +	struct stat st;
>  
> -	/*
> -	 * Parent creates a zero file using DIO.
> -	 * Truncates it to zero
> -	 * Create another file with '0xaa'
> -	 */
> -	for (i = 0; i < 100; i++) {
> -		dio_append(filename, 0);
> -		truncate(filename, 0);
> -		dio_append("junkfile", 0xaa);
> -		truncate("junkfile", 0);
> -	}
> +	if (SAFE_STAT(filename, &st) == 0)

SAFE_MACROS() exit the test on failure, that is the whole purpose of
them.

> +		return st.st_size;
> +
> +	return -1;
> +}
> +
> +static void run(void)
> +{
> +	int charnum = 0;
> +	long empty_ch = FILESIZE - STARTING_CHARS;
> +
> +	tst_res(TINFO, "Create %s filled with %d chars", FILENAME, STARTING_CHARS);
> +	dio_append(FILENAME, 'a', STARTING_CHARS, 1);
>  
> -	for (i = 0; i < num_children; i++) {
> -		kill(pid[i], SIGTERM);
> +	/* Truncate to a bigger file and check if it's filled with empty chars */
> +	tst_res(TINFO, "Truncate to %ld", FILESIZE);
> +	TST_EXP_POSITIVE(truncate(FILENAME, FILESIZE), "truncate(%s, %lu)", FILENAME, FILESIZE);

SAFE_TRUNCATE() here?

> +	if (!TST_PASS)
> +		return;
> +
> +	TEST(getsize(FILENAME));

The TEST() macro is supposed to be used in cases where we need to store
errno in order to make sure that it's not rewritten in subsequent system
calls. It's pointeles to use it here.

> +	if (TST_RET == FILESIZE) {
> +		tst_res(TPASS, "Truncated file has %ld length", TST_RET);
> +
> +		charnum = dio_get_zeros(FILENAME, FILESIZE);
> +
> +		if (charnum == empty_ch)
> +			tst_res(TPASS, "Truncated file provides %ld empty chars at the end", empty_ch);
> +		else
> +			tst_res(TFAIL, "Truncated file isn't filled with %i chars", charnum);
> +	} else {
> +		tst_res(TFAIL, "Truncated file doesn't have %ld length but %ld", FILESIZE, TST_RET);
>  	}
>  
> -	return 0;
> +	/* Truncate to zero: file must be empty */
> +	tst_res(TINFO, "Truncate to zero");
> +	TST_EXP_POSITIVE(truncate(FILENAME, 0), "truncate(%s, 0)", FILENAME);
> +	if (!TST_PASS)
> +		return;
> +
> +	TEST(getsize(FILENAME));
> +	if (TST_RET == 0)
> +		tst_res(TPASS, "Truncated file has zero length");
> +	else
> +		tst_res(TFAIL, "Truncated file doesn't have zero length");


Where are the children that do dio_read(FILENAME) on background while
the parent does this?

Also why don't we do N iterations of the test as we did before?

Also where is the part where we write to an unrelated file?

The point of this test is to check that the child processes read zeroes
using direct I/O when the parent does:

	in a loop:

	1. writes zeroes to the file being read
	2. truncates the file being read
	3. writes non-zero in an unrelated file
	4. truncates unrelated file

So the pass/success should be determined by the children and not the
parent here.

There are couple of solution there, but I guess that ideally:

* The child process will exit with 1 once it has found anything that is
  not zero

* The parent will check with waitpid() and WNOHANG if there is a child
  that has exitted in each loop iteration, if so kill children and fail the test

* If the parent is out of loops, kill the children and pass the test

>  }
> +
> +static struct tst_test test = {
> +	.needs_tmpdir = 1,
> +	.test_all = run,
> +};
> -- 
> 2.33.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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

[LTP] [PATCH v2] dio_truncate.c test refactory with LTP API

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

---
 testcases/kernel/io/ltp-aiodio/dio_truncate.c | 231 +++++++-----------
 1 file changed, 91 insertions(+), 140 deletions(-)

diff --git a/testcases/kernel/io/ltp-aiodio/dio_truncate.c b/testcases/kernel/io/ltp-aiodio/dio_truncate.c
index 27cf01525..10bad9454 100644
--- a/testcases/kernel/io/ltp-aiodio/dio_truncate.c
+++ b/testcases/kernel/io/ltp-aiodio/dio_truncate.c
@@ -1,177 +1,128 @@
-
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2004 Daniel McNeil <daniel@osdl.org>
- *               2004 Open Source Development Lab
- *   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
- *
- * Module: .c
+ *			    2004 Open Source Development Lab
+ *	Copyright (C) 2021 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
  */
 
-/*
- * Change History:
- *
- * 2/2004  Marty Ridgeway (mridge@us.ibm.com) Changes to adapt to LTP
- *
- */
 #define _GNU_SOURCE
 
 #include <stdlib.h>
+#include <stdio.h>
+#include <sys/stat.h>
 #include <sys/types.h>
-#include <signal.h>
-#include <errno.h>
 #include <fcntl.h>
-#include <stdio.h>
-#include <unistd.h>
-#include <memory.h>
-#include <string.h>
-#include <limits.h>
+#include "tst_test.h"
 
-#include "test.h"
+static char *FILENAME = "file.bin";
+static long FILESIZE = 64 * 1024;
+static int STARTING_CHARS = 10;
 
-#define NUM_CHILDREN 8
-
-char *check_zero(unsigned char *buf, int size)
+int dio_append(const char *path, char pattern, size_t bs, size_t bcount)
 {
-	unsigned char *p;
-
-	p = buf;
-
-	while (size > 0) {
-		if (*buf != 0) {
-			fprintf(stderr,
-				"non zero buffer at buf[%d] => 0x%02x,%02x,%02x,%02x\n",
-				buf - p, (unsigned int)buf[0],
-				size > 1 ? (unsigned int)buf[1] : 0,
-				size > 2 ? (unsigned int)buf[2] : 0,
-				size > 3 ? (unsigned int)buf[3] : 0);
-			fprintf(stderr, "buf %p, p %p\n", buf, p);
-			return buf;
-		}
-		buf++;
-		size--;
-	}
-	return 0;		/* all zeros */
-}
+	int fd = 0;
+	size_t i = 0;
+	char *buf = NULL;
 
-int dio_read(char *filename)
-{
-	int fd;
-	int r;
-	void *bufptr = NULL;
+	fd = SAFE_OPEN(path, O_CREAT|O_WRONLY|O_DIRECT, 0666);
 
-	TEST(posix_memalign(&bufptr, 4096, 64 * 1024));
-	if (TEST_RETURN) {
-		tst_resm(TBROK | TRERRNO, "cannot malloc aligned memory");
+	buf = (char *)SAFE_MEMALIGN(getpagesize(), bs);
+	if (buf == 0) {
+		SAFE_CLOSE(fd);
 		return -1;
 	}
 
-	while ((fd = open(filename, O_DIRECT | O_RDONLY)) < 0) {
-	}
-	fprintf(stderr, "dio_truncate: child reading file\n");
-	while (1) {
-		off_t offset;
-		char *bufoff;
-
-		/* read the file, checking for zeros */
-		offset = lseek(fd, SEEK_SET, 0);
-		do {
-			r = read(fd, bufptr, 64 * 1024);
-			if (r > 0) {
-				if ((bufoff = check_zero(bufptr, r))) {
-					fprintf(stderr,
-						"non-zero read at offset %p\n",
-						offset + bufoff);
-					exit(1);
-				}
-				offset += r;
-			}
-		} while (r > 0);
+	for (i = 0; i < bs; i++)
+		buf[i] = pattern;
+
+	for (i = 0; i < bcount; i++) {
+		if (write(fd, buf, bs) != (ssize_t)bs) {
+			free(buf);
+			return -1;
+		}
 	}
+
+	SAFE_CLOSE(fd);
+
 	return 0;
 }
 
-void dio_append(char *filename, int fill)
+int dio_get_zeros(const char *path, size_t bs)
 {
-	int fd;
+	int i = 0;
+	int fd = 0;
+	int zeros = 0;
 	void *bufptr = NULL;
-	int i;
-	int w;
 
-	fd = open(filename, O_DIRECT | O_WRONLY | O_CREAT, 0666);
+	fd = SAFE_OPEN(path, O_RDONLY|O_DIRECT, 0666);
 
-	if (fd < 0) {
-		perror("cannot create file");
-		return;
-	}
+	bufptr = SAFE_MALLOC(bs * sizeof(void));
+	SAFE_READ(0, fd, bufptr, bs);
 
-	TEST(posix_memalign(&bufptr, 4096, 64 * 1024));
-	if (TEST_RETURN) {
-		tst_resm(TBROK | TRERRNO, "cannot malloc aligned memory");
-		close(fd);
-		return;
+	for (i = 0; i < (int)bs; i++) {
+		if (*(char *)bufptr == 0)
+			zeros++;
+		bufptr++;
 	}
 
-	memset(bufptr, fill, 64 * 1024);
+	SAFE_CLOSE(fd);
 
-	for (i = 0; i < 1000; i++) {
-		if ((w = write(fd, bufptr, 64 * 1024)) != 64 * 1024) {
-			fprintf(stderr, "write %d returned %d\n", i, w);
-		}
-	}
-	close(fd);
+	return zeros;
 }
 
-int main(void)
+off_t getsize(const char *filename)
 {
-	char filename[PATH_MAX];
-	int pid[NUM_CHILDREN];
-	int num_children = 1;
-	int i;
-
-	snprintf(filename, sizeof(filename), "%s/aiodio/file",
-		 getenv("TMP") ? getenv("TMP") : "/tmp");
-
-	for (i = 0; i < num_children; i++) {
-		if ((pid[i] = fork()) == 0) {
-			/* child */
-			return dio_read(filename);
-		} else if (pid[i] < 0) {
-			/* error */
-			perror("fork error");
-			break;
-		} else {
-			/* Parent */
-			continue;
-		}
-	}
+	struct stat st;
 
-	/*
-	 * Parent creates a zero file using DIO.
-	 * Truncates it to zero
-	 * Create another file with '0xaa'
-	 */
-	for (i = 0; i < 100; i++) {
-		dio_append(filename, 0);
-		truncate(filename, 0);
-		dio_append("junkfile", 0xaa);
-		truncate("junkfile", 0);
-	}
+	if (SAFE_STAT(filename, &st) == 0)
+		return st.st_size;
+
+	return -1;
+}
+
+static void run(void)
+{
+	int charnum = 0;
+	long empty_ch = FILESIZE - STARTING_CHARS;
+
+	tst_res(TINFO, "Create %s filled with %d chars", FILENAME, STARTING_CHARS);
+	dio_append(FILENAME, 'a', STARTING_CHARS, 1);
 
-	for (i = 0; i < num_children; i++) {
-		kill(pid[i], SIGTERM);
+	/* Truncate to a bigger file and check if it's filled with empty chars */
+	tst_res(TINFO, "Truncate to %ld", FILESIZE);
+	TST_EXP_POSITIVE(truncate(FILENAME, FILESIZE), "truncate(%s, %lu)", FILENAME, FILESIZE);
+	if (!TST_PASS)
+		return;
+
+	TEST(getsize(FILENAME));
+
+	if (TST_RET == FILESIZE) {
+		tst_res(TPASS, "Truncated file has %ld length", TST_RET);
+
+		charnum = dio_get_zeros(FILENAME, FILESIZE);
+
+		if (charnum == empty_ch)
+			tst_res(TPASS, "Truncated file provides %ld empty chars at the end", empty_ch);
+		else
+			tst_res(TFAIL, "Truncated file isn't filled with %i chars", charnum);
+	} else {
+		tst_res(TFAIL, "Truncated file doesn't have %ld length but %ld", FILESIZE, TST_RET);
 	}
 
-	return 0;
+	/* Truncate to zero: file must be empty */
+	tst_res(TINFO, "Truncate to zero");
+	TST_EXP_POSITIVE(truncate(FILENAME, 0), "truncate(%s, 0)", FILENAME);
+	if (!TST_PASS)
+		return;
+
+	TEST(getsize(FILENAME));
+	if (TST_RET == 0)
+		tst_res(TPASS, "Truncated file has zero length");
+	else
+		tst_res(TFAIL, "Truncated file doesn't have zero length");
 }
+
+static struct tst_test test = {
+	.needs_tmpdir = 1,
+	.test_all = run,
+};
-- 
2.33.1


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