All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH] syscalls/getdents: don't mix libc and kernel types
@ 2020-11-16 12:43 j.nixdorf
  2020-11-18 13:58 ` Cyril Hrubis
  2020-11-19  8:44 ` [LTP] [PATCH v2] getdents: update to new api, " j.nixdorf
  0 siblings, 2 replies; 6+ messages in thread
From: j.nixdorf @ 2020-11-16 12:43 UTC (permalink / raw)
  To: ltp

Before this commit the tests called libc's getdents(64) with the kernel
types struct linux_dirent(64) as arguments, mixing libc functions and
kernel types.

This breaks on musl libc, as it provides getdents and struct dirent as
an alias to getdents64 and struct dirent64.

To not lose coverage we now test both supported configurations: the
syscall with the kernel types and the libc wrappers with the libc types.

Signed-off-by: Johannes Nixdorf <j.nixdorf@avm.de>
---
 runtest/syscalls                              |  6 ++
 testcases/kernel/syscalls/getdents/getdents.h | 34 +++++---
 .../kernel/syscalls/getdents/getdents01.c     | 74 +++++++++++++----
 .../kernel/syscalls/getdents/getdents02.c     | 79 +++++++++++++------
 travis/alpine.sh                              |  2 -
 5 files changed, 143 insertions(+), 52 deletions(-)

diff --git a/runtest/syscalls b/runtest/syscalls
index 0443f9f3d..97aec0a37 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -413,6 +413,12 @@ getdents02 getdents02
 getdents01_64 getdents01 -l
 getdents02_64 getdents02 -l
 
+getdents01_libc getdents01 -c
+getdents02_libc getdents02 -c
+
+getdents01_libc_64 getdents01 -cl
+getdents02_libc_64 getdents02 -cl
+
 getdomainname01 getdomainname01
 
 getdtablesize01 getdtablesize01
diff --git a/testcases/kernel/syscalls/getdents/getdents.h b/testcases/kernel/syscalls/getdents/getdents.h
index a20b1811a..19e2cd23f 100644
--- a/testcases/kernel/syscalls/getdents/getdents.h
+++ b/testcases/kernel/syscalls/getdents/getdents.h
@@ -24,6 +24,11 @@
 #include "test.h"
 #include "lapi/syscalls.h"
 #include "config.h"
+
+#if HAVE_GETDENTS || HAVE_GETDENTS64
+#include <unistd.h>
+#endif
+
 /*
  * See fs/compat.c struct compat_linux_dirent
  */
@@ -34,16 +39,19 @@ struct linux_dirent {
 	char            d_name[];
 };
 
-#if HAVE_GETDENTS
-#include <unistd.h>
-#else
 static inline int
-getdents(unsigned int fd, struct linux_dirent *dirp, unsigned int size)
+linux_getdents(unsigned int fd, struct linux_dirent *dirp, unsigned int size)
 {
 	return ltp_syscall(__NR_getdents, fd, dirp, size);
 }
 
-#endif /* HAVE_GETDENTS */
+#if HAVE_GETDENTS
+#define libc_getdents(fd, dirp, size) getdents(fd, dirp, size)
+#define libc_dirent dirent
+#else
+#define libc_getdents(fd, dirp, size) (-1)
+#define libc_dirent linux_dirent
+#endif
 
 struct linux_dirent64 {
 	uint64_t	d_ino;
@@ -53,14 +61,18 @@ struct linux_dirent64 {
 	char		d_name[];
 };
 
-#if HAVE_GETDENTS64
-#include <dirent.h>
-#include <unistd.h>
-#else
 static inline int
-getdents64(unsigned int fd, struct linux_dirent64 *dirp64, unsigned int size)
+linux_getdents64(unsigned int fd, struct linux_dirent64 *dirp64, unsigned int size)
 {
 	return ltp_syscall(__NR_getdents64, fd, dirp64, size);
 }
-#endif /* HAVE_GETDENTS64 */
+
+#if HAVE_GETDENTS64
+#define libc_getdents64(fd, dirp, size) getdents64(fd, dirp, size)
+#define libc_dirent64 dirent64
+#else
+#define libc_getdents64(fd, dirp, size) (-1)
+#define libc_dirent64 linux_dirent64
+#endif
+
 #endif /* GETDENTS_H */
diff --git a/testcases/kernel/syscalls/getdents/getdents01.c b/testcases/kernel/syscalls/getdents/getdents01.c
index eca572d42..0dfc81150 100644
--- a/testcases/kernel/syscalls/getdents/getdents01.c
+++ b/testcases/kernel/syscalls/getdents/getdents01.c
@@ -41,10 +41,12 @@ char *TCID = "getdents01";
 int TST_TOTAL = 1;
 
 static int longsyscall;
+static int libccall;
 
 static option_t options[] = {
 		/* -l long option. Tests getdents64 */
 		{"l", &longsyscall, NULL},
+		{"c", &libccall, NULL},
 		{NULL, NULL, NULL}
 };
 
@@ -84,20 +86,34 @@ int main(int ac, char **av)
 {
 	int lc;
 	int rval, fd;
-	struct linux_dirent64 *dirp64;
-	struct linux_dirent *dirp;
+	struct linux_dirent64 *linux_dirp64;
+	struct linux_dirent *linux_dirp;
+	struct libc_dirent64 *libc_dirp64;
+	struct libc_dirent *libc_dirp;
 	void *buf;
 
 	tst_parse_opts(ac, av, options, &help);
 
+#if !HAVE_GETDENTS
+	if (libccall && !longsyscall)
+		tst_brkm(TCONF, NULL, "Unsupported libc function: getdents");
+#endif
+
+#if !HAVE_GETDENTS64
+	if (libccall && longsyscall)
+		tst_brkm(TCONF, NULL, "Unsupported libc function: getdents64");
+#endif
+
 	/* The buffer is allocated to make sure it's suitably aligned */
 	buf = malloc(BUFSIZE);
 
 	if (buf == NULL)
 		tst_brkm(TBROK, NULL, "malloc failed");
 
-	dirp64 = buf;
-	dirp = buf;
+	linux_dirp64 = buf;
+	linux_dirp = buf;
+	libc_dirp64 = buf;
+	libc_dirp = buf;
 
 	setup();
 
@@ -109,10 +125,17 @@ int main(int ac, char **av)
 		if ((fd = open(".", O_RDONLY)) == -1)
 			tst_brkm(TBROK, cleanup, "open of directory failed");
 
-		if (longsyscall)
-			rval = getdents64(fd, dirp64, BUFSIZE);
-		else
-			rval = getdents(fd, dirp, BUFSIZE);
+		if (libccall) {
+			if (longsyscall)
+				rval = libc_getdents64(fd, libc_dirp64, BUFSIZE);
+			else
+				rval = libc_getdents(fd, libc_dirp, BUFSIZE);
+		} else {
+			if (longsyscall)
+				rval = linux_getdents64(fd, linux_dirp64, BUFSIZE);
+			else
+				rval = linux_getdents(fd, linux_dirp, BUFSIZE);
+		}
 
 		if (rval < 0) {
 			if (errno == ENOSYS)
@@ -134,12 +157,22 @@ int main(int ac, char **av)
 		do {
 			size_t d_reclen;
 
-			if (longsyscall) {
-				d_reclen = dirp64->d_reclen;
-				d_name = dirp64->d_name;
+			if (libccall) {
+				if (longsyscall) {
+					d_reclen = libc_dirp64->d_reclen;
+					d_name = libc_dirp64->d_name;
+				} else {
+					d_reclen = libc_dirp->d_reclen;
+					d_name = libc_dirp->d_name;
+				}
 			} else {
-				d_reclen = dirp->d_reclen;
-				d_name = dirp->d_name;
+				if (longsyscall) {
+					d_reclen = linux_dirp64->d_reclen;
+					d_name = linux_dirp64->d_name;
+				} else {
+					d_reclen = linux_dirp->d_reclen;
+					d_name = linux_dirp->d_name;
+				}
 			}
 
 			set_flag(d_name);
@@ -148,10 +181,17 @@ int main(int ac, char **av)
 			
 			rval -= d_reclen;
 			
-			if (longsyscall)
-				dirp64 = (void*)dirp64 + d_reclen;
-			else
-				dirp = (void*)dirp + d_reclen;
+			if (libccall) {
+				if (longsyscall)
+					libc_dirp64 = (void *)libc_dirp64 + d_reclen;
+				else
+					libc_dirp = (void *)libc_dirp + d_reclen;
+			} else {
+				if (longsyscall)
+					linux_dirp64 = (void *)linux_dirp64 + d_reclen;
+				else
+					linux_dirp = (void *)linux_dirp + d_reclen;
+			}
 
 		} while (rval > 0);
 
diff --git a/testcases/kernel/syscalls/getdents/getdents02.c b/testcases/kernel/syscalls/getdents/getdents02.c
index 4cf54aea3..71ea6a362 100644
--- a/testcases/kernel/syscalls/getdents/getdents02.c
+++ b/testcases/kernel/syscalls/getdents/getdents02.c
@@ -65,16 +65,19 @@ static void (*testfunc[])(void) = { test_ebadf, test_einval,
 int TST_TOTAL = ARRAY_SIZE(testfunc);
 
 static int longsyscall;
+static int libccall;
 
 option_t options[] = {
 		/* -l long option. Tests getdents64 */
 		{"l", &longsyscall, NULL},
+		{"c", &libccall, NULL},
 		{NULL, NULL, NULL}
 };
 
 static void help(void)
 {
 	printf("  -l      Test the getdents64 system call\n");
+	printf("  -c      Test the libc wrappers around the getdents(64) system call\n");
 }
 
 int main(int ac, char **av)
@@ -83,6 +86,16 @@ int main(int ac, char **av)
 
 	tst_parse_opts(ac, av, options, &help);
 
+#if !HAVE_GETDENTS
+	if (libccall && !longsyscall)
+		tst_brkm(TCONF, NULL, "Unsupported libc function: getdents");
+#endif
+
+#if !HAVE_GETDENTS64
+	if (libccall && longsyscall)
+		tst_brkm(TCONF, NULL, "Unsupported libc function: getdents64");
+#endif
+
 	setup();
 
 	for (lc = 0; TEST_LOOPING(lc); lc++) {
@@ -120,16 +133,51 @@ static void print_test_result(int err, int exp_errno)
 	}
 }
 
+static int test_getdents(int fd, void *dirp, size_t len, int stack)
+{
+	struct linux_dirent64 linux_dirp64;
+	struct linux_dirent linux_dirp;
+	struct libc_dirent64 libc_dirp64;
+	struct libc_dirent libc_dirp;
+
+	if (stack) {
+		if (libccall) {
+			if (longsyscall) {
+				dirp = &libc_dirp64;
+				len = sizeof(libc_dirp64);
+			} else {
+				dirp = &libc_dirp;
+				len = sizeof(libc_dirp);
+			}
+		} else {
+			if (longsyscall) {
+				dirp = &linux_dirp64;
+				len = sizeof(linux_dirp64);
+			} else {
+				dirp = &linux_dirp;
+				len = sizeof(linux_dirp);
+			}
+		}
+	}
+
+	if (libccall) {
+		if (longsyscall)
+			return libc_getdents64(fd, dirp, len);
+		else
+			return libc_getdents(fd, dirp, len);
+	} else {
+		if (longsyscall)
+			return linux_getdents64(fd, dirp, len);
+		else
+			return linux_getdents(fd, dirp, len);
+	}
+}
+
 static void test_ebadf(void)
 {
 	int fd = -5;
-	struct linux_dirent64 dirp64;
-	struct linux_dirent dirp;
 
-	if (longsyscall)
-		getdents64(fd, &dirp64, sizeof(dirp64));
-	else
-		getdents(fd, &dirp, sizeof(dirp));
+	test_getdents(fd, NULL, 0, 1);
 
 	print_test_result(errno, EBADF);
 }
@@ -142,10 +190,7 @@ static void test_einval(void)
 	fd = SAFE_OPEN(cleanup, ".", O_RDONLY);
 
 	/* Pass one byte long buffer. The result should be EINVAL */
-	if (longsyscall)
-		getdents64(fd, (void *)buf, sizeof(buf));
-	else
-		getdents(fd, (void *)buf, sizeof(buf));
+	test_getdents(fd, (void *)buf, sizeof(buf), 0);
 
 	print_test_result(errno, EINVAL);
 
@@ -155,15 +200,10 @@ static void test_einval(void)
 static void test_enotdir(void)
 {
 	int fd;
-	struct linux_dirent64 dir64;
-	struct linux_dirent dir;
 
 	fd = SAFE_OPEN(cleanup, "test", O_CREAT | O_RDWR, 0644);
 
-	if (longsyscall)
-		getdents64(fd, &dir64, sizeof(dir64));
-	else
-		getdents(fd, &dir, sizeof(dir));
+	test_getdents(fd, NULL, 0, 1);
 
 	print_test_result(errno, ENOTDIR);
 
@@ -173,18 +213,13 @@ static void test_enotdir(void)
 static void test_enoent(void)
 {
 	int fd;
-	struct linux_dirent64 dir64;
-	struct linux_dirent dir;
 
 	SAFE_MKDIR(cleanup, TEST_DIR, DIR_MODE);
 
 	fd = SAFE_OPEN(cleanup, TEST_DIR, O_DIRECTORY);
 	SAFE_RMDIR(cleanup, TEST_DIR);
 
-	if (longsyscall)
-		getdents64(fd, &dir64, sizeof(dir64));
-	else
-		getdents(fd, &dir, sizeof(dir));
+	test_getdents(fd, NULL, 0, 1);
 
 	print_test_result(errno, ENOENT);
 
diff --git a/travis/alpine.sh b/travis/alpine.sh
index 61ef144a8..ffd5a8ebc 100755
--- a/travis/alpine.sh
+++ b/travis/alpine.sh
@@ -33,8 +33,6 @@ rm -rfv \
 	testcases/kernel/syscalls/confstr/confstr01.c \
 	testcases/kernel/syscalls/fmtmsg/fmtmsg01.c \
 	testcases/kernel/syscalls/getcontext/getcontext01.c \
-	testcases/kernel/syscalls/getdents/getdents01.c \
-	testcases/kernel/syscalls/getdents/getdents02.c \
 	testcases/kernel/syscalls/rt_tgsigqueueinfo/rt_tgsigqueueinfo01.c \
 	testcases/kernel/syscalls/timer_create/timer_create01.c \
 	testcases/kernel/syscalls/timer_create/timer_create03.c \
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [LTP] [PATCH] syscalls/getdents: don't mix libc and kernel types
  2020-11-16 12:43 [LTP] [PATCH] syscalls/getdents: don't mix libc and kernel types j.nixdorf
@ 2020-11-18 13:58 ` Cyril Hrubis
  2020-11-19  8:44 ` [LTP] [PATCH v2] getdents: update to new api, " j.nixdorf
  1 sibling, 0 replies; 6+ messages in thread
From: Cyril Hrubis @ 2020-11-18 13:58 UTC (permalink / raw)
  To: ltp

Hi!
First of all it would be easier to convert these testcases to the new
test library first since aparat from other things the new library has
support for test variants so that we can run one test for different
syscall wrappers.

Have a look for example at syscalls/stime/ how this could be
implemented.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [LTP] [PATCH v2] getdents: update to new api, don't mix libc and kernel types
  2020-11-16 12:43 [LTP] [PATCH] syscalls/getdents: don't mix libc and kernel types j.nixdorf
  2020-11-18 13:58 ` Cyril Hrubis
@ 2020-11-19  8:44 ` j.nixdorf
  2020-11-19 13:00   ` Cyril Hrubis
  2020-11-20 11:56   ` [LTP] [PATCH v3] getdents: update to the " j.nixdorf
  1 sibling, 2 replies; 6+ messages in thread
From: j.nixdorf @ 2020-11-19  8:44 UTC (permalink / raw)
  To: ltp

Before this commit the tests called libc's getdents(64) with the kernel
types struct linux_dirent(64) as arguments, mixing libc functions and
kernel types.

This breaks on musl libc, as it provides getdents and struct dirent as
an alias to getdents64 and struct dirent64.

To not lose coverage we now test both supported configurations: the
syscall with the kernel types and the libc wrappers with the libc types.

To take advantage of support for multiple test_variants the tests were
converted to the new test API at the same time.

Signed-off-by: Johannes Nixdorf <j.nixdorf@avm.de>

---

v2: Converted to the new test API, using test_variants for all libc and
    syscall variants.

 runtest/syscalls                              |   3 -
 testcases/kernel/syscalls/getdents/getdents.h | 162 +++++++++++++--
 .../kernel/syscalls/getdents/getdents01.c     | 191 +++++++-----------
 .../kernel/syscalls/getdents/getdents02.c     | 111 +++-------
 travis/alpine.sh                              |   2 -
 5 files changed, 248 insertions(+), 221 deletions(-)

diff --git a/runtest/syscalls b/runtest/syscalls
index 0443f9f3d..8f2fe0db2 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -410,9 +410,6 @@ getcwd04 getcwd04
 getdents01 getdents01
 getdents02 getdents02
 
-getdents01_64 getdents01 -l
-getdents02_64 getdents02 -l
-
 getdomainname01 getdomainname01
 
 getdtablesize01 getdtablesize01
diff --git a/testcases/kernel/syscalls/getdents/getdents.h b/testcases/kernel/syscalls/getdents/getdents.h
index a20b1811a..9a4f88d7d 100644
--- a/testcases/kernel/syscalls/getdents/getdents.h
+++ b/testcases/kernel/syscalls/getdents/getdents.h
@@ -21,9 +21,13 @@
 #define GETDENTS_H
 
 #include <stdint.h>
-#include "test.h"
-#include "lapi/syscalls.h"
 #include "config.h"
+#include "lapi/syscalls.h"
+
+#if HAVE_GETDENTS || HAVE_GETDENTS64
+#include <unistd.h>
+#endif
+
 /*
  * See fs/compat.c struct compat_linux_dirent
  */
@@ -34,17 +38,12 @@ struct linux_dirent {
 	char            d_name[];
 };
 
-#if HAVE_GETDENTS
-#include <unistd.h>
-#else
 static inline int
-getdents(unsigned int fd, struct linux_dirent *dirp, unsigned int size)
+linux_getdents(unsigned int fd, struct linux_dirent *dirp, unsigned int size)
 {
-	return ltp_syscall(__NR_getdents, fd, dirp, size);
+	return tst_syscall(__NR_getdents, fd, dirp, size);
 }
 
-#endif /* HAVE_GETDENTS */
-
 struct linux_dirent64 {
 	uint64_t	d_ino;
 	int64_t		d_off;
@@ -53,14 +52,149 @@ struct linux_dirent64 {
 	char		d_name[];
 };
 
+static inline int
+linux_getdents64(unsigned int fd, struct linux_dirent64 *dirp64, unsigned int size)
+{
+	return tst_syscall(__NR_getdents64, fd, dirp64, size);
+}
+
+static inline size_t
+tst_dirp_size(void)
+{
+	switch (tst_variant) {
+	case 0:
+		return sizeof(struct linux_dirent);
+	case 1:
+		return sizeof(struct linux_dirent64);
+	case 2:
+#if HAVE_GETDENTS
+		return sizeof(struct dirent);
+#else
+		tst_brk(TCONF, "libc getdents() is not implemented");
+		break;
+#endif
+	case 3:
 #if HAVE_GETDENTS64
-#include <dirent.h>
-#include <unistd.h>
+		return sizeof(struct dirent64);
+#else
+		tst_brk(TCONF, "libc getdents64() is not implemented");
+		break;
+#endif
+	}
+	return 0;
+}
+
+static inline void *
+tst_dirp_alloc(void)
+{
+	return tst_alloc(tst_dirp_size());
+}
+
+static inline const char *
+tst_dirp_name(void *dirp)
+{
+	switch (tst_variant) {
+	case 0:
+		return ((struct linux_dirent *)dirp)->d_name;
+	case 1:
+		return ((struct linux_dirent64 *)dirp)->d_name;
+	case 2:
+#if HAVE_GETDENTS
+		return ((struct dirent *)dirp)->d_name;
 #else
+		tst_brk(TCONF, "libc getdents() is not implemented");
+		break;
+#endif
+	case 3:
+#if HAVE_GETDENTS64
+		return ((struct dirent64 *)dirp)->d_name;
+#else
+		tst_brk(TCONF, "libc getdents64() is not implemented");
+		break;
+#endif
+	}
+	return NULL;
+}
+
+static inline size_t
+tst_dirp_reclen(void *dirp)
+{
+	switch (tst_variant) {
+	case 0:
+		return ((struct linux_dirent *)dirp)->d_reclen;
+	case 1:
+		return ((struct linux_dirent64 *)dirp)->d_reclen;
+	case 2:
+#if HAVE_GETDENTS
+		return ((struct dirent *)dirp)->d_reclen;
+#else
+		tst_brk(TCONF, "libc getdents() is not implemented");
+		break;
+#endif
+	case 3:
+#if HAVE_GETDENTS64
+		return ((struct dirent64 *)dirp)->d_reclen;
+#else
+		tst_brk(TCONF, "libc getdents64() is not implemented");
+		break;
+#endif
+	}
+	return 0;
+}
+
 static inline int
-getdents64(unsigned int fd, struct linux_dirent64 *dirp64, unsigned int size)
+tst_getdents(int fd, void *dirp, unsigned int size)
+{
+	switch (tst_variant) {
+	case 0:
+		return linux_getdents(fd, dirp, size);
+	case 1:
+		return linux_getdents64(fd, dirp, size);
+	case 2:
+#if HAVE_GETDENTS
+		return getdents(fd, dirp, size);
+#else
+		tst_brk(TCONF, "libc getdents() is not implemented");
+		break;
+#endif
+	case 3:
+#if HAVE_GETDENTS64
+		return getdents64(fd, dirp, size);
+#else
+		tst_brk(TCONF, "libc getdents64() is not implemented");
+		break;
+#endif
+	}
+	return -1;
+}
+
+static inline void
+getdents_info(void)
 {
-	return ltp_syscall(__NR_getdents64, fd, dirp64, size);
+	switch (tst_variant) {
+	case 0:
+		tst_res(TINFO, "Testing the SYS_getdents syscall");
+		break;
+	case 1:
+		tst_res(TINFO, "Testing the SYS_getdents64 syscall");
+		break;
+	case 2:
+#if HAVE_GETDENTS
+		tst_res(TINFO, "Testing libc getdents()");
+#else
+		tst_brk(TCONF, "libc getdents() is not implemented");
+#endif
+		break;
+	case 3:
+#if HAVE_GETDENTS64
+		tst_res(TINFO, "Testing libc getdents64()");
+#else
+		tst_brk(TCONF, "libc getdents64() is not implemented");
+#endif
+		break;
+	}
 }
-#endif /* HAVE_GETDENTS64 */
+
+#define TEST_VARIANTS 4
+
 #endif /* GETDENTS_H */
diff --git a/testcases/kernel/syscalls/getdents/getdents01.c b/testcases/kernel/syscalls/getdents/getdents01.c
index eca572d42..bdf939ab0 100644
--- a/testcases/kernel/syscalls/getdents/getdents01.c
+++ b/testcases/kernel/syscalls/getdents/getdents01.c
@@ -20,16 +20,12 @@
  */
 
 #define _GNU_SOURCE
-#include <stdio.h>
-#include <errno.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-
-#include "test.h"
-#include "safe_macros.h"
+
+#include "tst_test.h"
 #include "getdents.h"
 
+#include <stdlib.h>
+
 static void cleanup(void);
 static void setup(void);
 
@@ -37,21 +33,7 @@ static void reset_flags(void);
 static void check_flags(void);
 static void set_flag(const char *name);
 
-char *TCID = "getdents01";
-int TST_TOTAL = 1;
-
-static int longsyscall;
-
-static option_t options[] = {
-		/* -l long option. Tests getdents64 */
-		{"l", &longsyscall, NULL},
-		{NULL, NULL, NULL}
-};
-
-static void help(void)
-{
-	printf("  -l      Test the getdents64 system call\n");
-}
+static int fd;
 
 enum entry_type {
 	ENTRY_DIR,
@@ -80,95 +62,49 @@ struct testcase testcases[] = {
  */
 #define BUFSIZE 512
 
-int main(int ac, char **av)
+void run(void)
 {
-	int lc;
-	int rval, fd;
-	struct linux_dirent64 *dirp64;
-	struct linux_dirent *dirp;
-	void *buf;
-
-	tst_parse_opts(ac, av, options, &help);
+	int rval;
+	void *dirp;
 
 	/* The buffer is allocated to make sure it's suitably aligned */
-	buf = malloc(BUFSIZE);
-
-	if (buf == NULL)
-		tst_brkm(TBROK, NULL, "malloc failed");
-
-	dirp64 = buf;
-	dirp = buf;
+	dirp = tst_alloc(BUFSIZE);
 
-	setup();
+	if (dirp == NULL)
+		tst_brk(TBROK, "malloc failed");
 
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		const char *d_name;
+	rval = tst_getdents(fd, dirp, BUFSIZE);
 
-		tst_count = 0;
-
-		if ((fd = open(".", O_RDONLY)) == -1)
-			tst_brkm(TBROK, cleanup, "open of directory failed");
-
-		if (longsyscall)
-			rval = getdents64(fd, dirp64, BUFSIZE);
+	if (rval < 0) {
+		if (errno == ENOSYS)
+			tst_brk(TCONF, "syscall not implemented");
 		else
-			rval = getdents(fd, dirp, BUFSIZE);
-
-		if (rval < 0) {
-			if (errno == ENOSYS)
-				tst_resm(TCONF, "syscall not implemented");
-			else
-				tst_resm(TFAIL | TERRNO,
-				         "getdents failed unexpectedly");
-			continue;
-		}
-
-		if (rval == 0) {
-			tst_resm(TFAIL,
-				 "getdents failed - returned end of directory");
-			continue;
-		}
-
-		reset_flags();
-
-		do {
-			size_t d_reclen;
+			tst_brk(TFAIL | TERRNO, "getdents failed unexpectedly");
+	}
 
-			if (longsyscall) {
-				d_reclen = dirp64->d_reclen;
-				d_name = dirp64->d_name;
-			} else {
-				d_reclen = dirp->d_reclen;
-				d_name = dirp->d_name;
-			}
+	if (rval == 0)
+		tst_brk(TFAIL, "getdents failed - returned end of directory");
 
-			set_flag(d_name);
+	reset_flags();
 
-			tst_resm(TINFO, "Found '%s'", d_name);
-			
-			rval -= d_reclen;
-			
-			if (longsyscall)
-				dirp64 = (void*)dirp64 + d_reclen;
-			else
-				dirp = (void*)dirp + d_reclen;
+	do {
+		size_t d_reclen = tst_dirp_reclen(dirp);
+		const char *d_name = tst_dirp_name(dirp);
 
-		} while (rval > 0);
+		set_flag(d_name);
 
-		SAFE_CLOSE(cleanup, fd);
-	
-		check_flags();
-	}
+		tst_res(TINFO, "Found '%s'", d_name);
 
-	free(buf);
+		rval -= d_reclen;
+		dirp += d_reclen;
+	} while (rval > 0);
 
-	cleanup();
-	tst_exit();
+	check_flags();
 }
 
 static void reset_flags(void)
 {
-	int i;
+	size_t i;
 
 	for (i = 0; i < ARRAY_SIZE(testcases); i++)
 		testcases[i].found = 0;
@@ -176,24 +112,25 @@ static void reset_flags(void)
 
 static void check_flags(void)
 {
-	int i, err = 0;
+	size_t i;
+	int err = 0;
 
 	for (i = 0; i < ARRAY_SIZE(testcases); i++) {
 		if (!testcases[i].found) {
-			tst_resm(TINFO, "Entry '%s' not found", testcases[i].name);
+			tst_res(TINFO, "Entry '%s' not found", testcases[i].name);
 			err++;
 		}
 	}
 
 	if (err)
-		tst_resm(TFAIL, "Some entires not found");
+		tst_res(TFAIL, "Some entries not found");
 	else
-		tst_resm(TPASS, "All entires found");
+		tst_res(TPASS, "All entries found");
 }
 
 static void set_flag(const char *name)
 {
-	int i;
+	size_t i;
 
 	for (i = 0; i < ARRAY_SIZE(testcases); i++) {
 		if (!strcmp(name, testcases[i].name)) {
@@ -202,39 +139,47 @@ static void set_flag(const char *name)
 		}
 	}
 
-	tst_resm(TFAIL, "Unexpected entry '%s' found", name);
+	tst_res(TFAIL, "Unexpected entry '%s' found", name);
 }
 
 static void setup(void)
 {
-	int i;
-
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
-	tst_tmpdir();
-
-	for (i = 0; i < ARRAY_SIZE(testcases); i++) {
-		
-		if (!testcases[i].create)
-			continue;
-	
-		switch (testcases[i].type) {
-		case ENTRY_DIR:
-			SAFE_MKDIR(cleanup, testcases[i].name, 0777);
-		break;
-		case ENTRY_FILE:
-			SAFE_FILE_PRINTF(cleanup, testcases[i].name, " ");
-		break;
-		case ENTRY_SYMLINK:
-			SAFE_SYMLINK(cleanup, "nonexistent", testcases[i].name);
-		break;
+	size_t i;
+
+	getdents_info();
+
+	if (!tst_variant) {
+		for (i = 0; i < ARRAY_SIZE(testcases); i++) {
+			if (!testcases[i].create)
+				continue;
+
+			switch (testcases[i].type) {
+			case ENTRY_DIR:
+				SAFE_MKDIR(testcases[i].name, 0777);
+			break;
+			case ENTRY_FILE:
+				SAFE_FILE_PRINTF(testcases[i].name, " ");
+			break;
+			case ENTRY_SYMLINK:
+				SAFE_SYMLINK("nonexistent", testcases[i].name);
+			break;
+			}
 		}
 	}
 
-	TEST_PAUSE;
+	fd = SAFE_OPEN(".", O_RDONLY|O_DIRECTORY);
 }
 
 static void cleanup(void)
 {
-	tst_rmdir();
+	if (fd != 0)
+		SAFE_CLOSE(fd);
 }
+
+static struct tst_test test = {
+	.needs_tmpdir = 1,
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_variants = TEST_VARIANTS,
+};
diff --git a/testcases/kernel/syscalls/getdents/getdents02.c b/testcases/kernel/syscalls/getdents/getdents02.c
index 4cf54aea3..82932b7b1 100644
--- a/testcases/kernel/syscalls/getdents/getdents02.c
+++ b/testcases/kernel/syscalls/getdents/getdents02.c
@@ -34,22 +34,15 @@
  */
 
 #define _GNU_SOURCE
-#include <stdio.h>
 #include <errno.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
 
-#include "test.h"
+#include "tst_test.h"
 #include "getdents.h"
-#include "safe_macros.h"
 
 #define DIR_MODE	(S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP| \
 			 S_IXGRP|S_IROTH|S_IXOTH)
 #define TEST_DIR	"test_dir"
 
-static void cleanup(void);
-static void setup(void);
 static void print_test_result(int err, int exp_errno);
 
 char *TCID = "getdents02";
@@ -64,58 +57,33 @@ static void (*testfunc[])(void) = { test_ebadf, test_einval,
 
 int TST_TOTAL = ARRAY_SIZE(testfunc);
 
-static int longsyscall;
+static void *dirp;
+static size_t size;
 
-option_t options[] = {
-		/* -l long option. Tests getdents64 */
-		{"l", &longsyscall, NULL},
-		{NULL, NULL, NULL}
-};
-
-static void help(void)
-{
-	printf("  -l      Test the getdents64 system call\n");
-}
-
-int main(int ac, char **av)
+static void setup(void)
 {
-	int lc, i;
-
-	tst_parse_opts(ac, av, options, &help);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
+	getdents_info();
 
-		for (i = 0; i < TST_TOTAL; i++)
-			(*testfunc[i])();
-	}
-
-	cleanup();
-	tst_exit();
+	dirp = tst_dirp_alloc();
+	size = tst_dirp_size();
 }
 
-static void setup(void)
+static void run(unsigned int i)
 {
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
-	tst_tmpdir();
-
-	TEST_PAUSE;
+	testfunc[i]();
 }
 
 static void print_test_result(int err, int exp_errno)
 {
 	if (err == 0) {
-		tst_resm(TFAIL, "call succeeded unexpectedly");
+		tst_res(TFAIL, "call succeeded unexpectedly");
 	} else if  (err == exp_errno) {
-		tst_resm(TPASS, "getdents failed as expected: %s",
+		tst_res(TPASS, "getdents failed as expected: %s",
 			 strerror(err));
 	} else if (err == ENOSYS) {
-		tst_resm(TCONF, "syscall not implemented");
+		tst_res(TCONF, "syscall not implemented");
 	} else {
-		tst_resm(TFAIL, "getdents failed unexpectedly: %s",
+		tst_res(TFAIL, "getdents failed unexpectedly: %s",
 			 strerror(err));
 	}
 }
@@ -123,13 +91,8 @@ static void print_test_result(int err, int exp_errno)
 static void test_ebadf(void)
 {
 	int fd = -5;
-	struct linux_dirent64 dirp64;
-	struct linux_dirent dirp;
 
-	if (longsyscall)
-		getdents64(fd, &dirp64, sizeof(dirp64));
-	else
-		getdents(fd, &dirp, sizeof(dirp));
+	tst_getdents(fd, dirp, size);
 
 	print_test_result(errno, EBADF);
 }
@@ -139,59 +102,49 @@ static void test_einval(void)
 	int fd;
 	char buf[1];
 
-	fd = SAFE_OPEN(cleanup, ".", O_RDONLY);
+	fd = SAFE_OPEN(".", O_RDONLY);
 
 	/* Pass one byte long buffer. The result should be EINVAL */
-	if (longsyscall)
-		getdents64(fd, (void *)buf, sizeof(buf));
-	else
-		getdents(fd, (void *)buf, sizeof(buf));
+	tst_getdents(fd, (void *)buf, sizeof(buf));
 
 	print_test_result(errno, EINVAL);
 
-	SAFE_CLOSE(cleanup, fd);
+	SAFE_CLOSE(fd);
 }
 
 static void test_enotdir(void)
 {
 	int fd;
-	struct linux_dirent64 dir64;
-	struct linux_dirent dir;
 
-	fd = SAFE_OPEN(cleanup, "test", O_CREAT | O_RDWR, 0644);
+	fd = SAFE_OPEN("test", O_CREAT | O_RDWR, 0644);
 
-	if (longsyscall)
-		getdents64(fd, &dir64, sizeof(dir64));
-	else
-		getdents(fd, &dir, sizeof(dir));
+	tst_getdents(fd, dirp, size);
 
 	print_test_result(errno, ENOTDIR);
 
-	SAFE_CLOSE(cleanup, fd);
+	SAFE_CLOSE(fd);
 }
 
 static void test_enoent(void)
 {
 	int fd;
-	struct linux_dirent64 dir64;
-	struct linux_dirent dir;
 
-	SAFE_MKDIR(cleanup, TEST_DIR, DIR_MODE);
+	SAFE_MKDIR(TEST_DIR, DIR_MODE);
 
-	fd = SAFE_OPEN(cleanup, TEST_DIR, O_DIRECTORY);
-	SAFE_RMDIR(cleanup, TEST_DIR);
+	fd = SAFE_OPEN(TEST_DIR, O_DIRECTORY);
+	SAFE_RMDIR(TEST_DIR);
 
-	if (longsyscall)
-		getdents64(fd, &dir64, sizeof(dir64));
-	else
-		getdents(fd, &dir, sizeof(dir));
+	tst_getdents(fd, dirp, size);
 
 	print_test_result(errno, ENOENT);
 
-	SAFE_CLOSE(cleanup, fd);
+	SAFE_CLOSE(fd);
 }
 
-static void cleanup(void)
-{
-	tst_rmdir();
-}
+static struct tst_test test = {
+	.needs_tmpdir = 1,
+	.test = run,
+	.setup = setup,
+	.tcnt = ARRAY_SIZE(testfunc),
+	.test_variants = TEST_VARIANTS,
+};
diff --git a/travis/alpine.sh b/travis/alpine.sh
index 61ef144a8..ffd5a8ebc 100755
--- a/travis/alpine.sh
+++ b/travis/alpine.sh
@@ -33,8 +33,6 @@ rm -rfv \
 	testcases/kernel/syscalls/confstr/confstr01.c \
 	testcases/kernel/syscalls/fmtmsg/fmtmsg01.c \
 	testcases/kernel/syscalls/getcontext/getcontext01.c \
-	testcases/kernel/syscalls/getdents/getdents01.c \
-	testcases/kernel/syscalls/getdents/getdents02.c \
 	testcases/kernel/syscalls/rt_tgsigqueueinfo/rt_tgsigqueueinfo01.c \
 	testcases/kernel/syscalls/timer_create/timer_create01.c \
 	testcases/kernel/syscalls/timer_create/timer_create03.c \
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [LTP] [PATCH v2] getdents: update to new api, don't mix libc and kernel types
  2020-11-19  8:44 ` [LTP] [PATCH v2] getdents: update to new api, " j.nixdorf
@ 2020-11-19 13:00   ` Cyril Hrubis
  2020-11-20 11:56   ` [LTP] [PATCH v3] getdents: update to the " j.nixdorf
  1 sibling, 0 replies; 6+ messages in thread
From: Cyril Hrubis @ 2020-11-19 13:00 UTC (permalink / raw)
  To: ltp

Hi!
>  #include <stdint.h>
> -#include "test.h"
> -#include "lapi/syscalls.h"
>  #include "config.h"
> +#include "lapi/syscalls.h"
> +
> +#if HAVE_GETDENTS || HAVE_GETDENTS64
> +#include <unistd.h>
> +#endif
> +
>  /*
>   * See fs/compat.c struct compat_linux_dirent
>   */
> @@ -34,17 +38,12 @@ struct linux_dirent {
>  	char            d_name[];
>  };
>  
> -#if HAVE_GETDENTS
> -#include <unistd.h>
> -#else
>  static inline int
> -getdents(unsigned int fd, struct linux_dirent *dirp, unsigned int size)
> +linux_getdents(unsigned int fd, struct linux_dirent *dirp, unsigned int size)
>  {
> -	return ltp_syscall(__NR_getdents, fd, dirp, size);
> +	return tst_syscall(__NR_getdents, fd, dirp, size);
>  }
>  
> -#endif /* HAVE_GETDENTS */
> -
>  struct linux_dirent64 {
>  	uint64_t	d_ino;
>  	int64_t		d_off;
> @@ -53,14 +52,149 @@ struct linux_dirent64 {
>  	char		d_name[];
>  };
>  
> +static inline int
> +linux_getdents64(unsigned int fd, struct linux_dirent64 *dirp64, unsigned int size)
> +{
> +	return tst_syscall(__NR_getdents64, fd, dirp64, size);
> +}
> +
> +static inline size_t
> +tst_dirp_size(void)
> +{
> +	switch (tst_variant) {
> +	case 0:
> +		return sizeof(struct linux_dirent);
> +	case 1:
> +		return sizeof(struct linux_dirent64);
> +	case 2:
> +#if HAVE_GETDENTS
> +		return sizeof(struct dirent);
> +#else
> +		tst_brk(TCONF, "libc getdents() is not implemented");
> +		break;
> +#endif
> +	case 3:
>  #if HAVE_GETDENTS64
> -#include <dirent.h>
> -#include <unistd.h>
> +		return sizeof(struct dirent64);
> +#else
> +		tst_brk(TCONF, "libc getdents64() is not implemented");
> +		break;

Well we do call the getdents_info() as a first thing in the test
setup() so I would say that we do not have to stuff tst_brk(TCONF, ...)
to all of the wrappers here since the test will exit before these
function are reached.

So maybe we can just do return 0; here and be done with it.

> +#endif
> +	}
> +	return 0;
> +}
> +
> +static inline void *
> +tst_dirp_alloc(void)
> +{
> +	return tst_alloc(tst_dirp_size());
> +}

Can we please avoid adding this wrapper?

We do call tst_dirp_size() in the setup anyways, so passing the size we
get to tst_alloc() is not more complicated than this.

> +static inline const char *
> +tst_dirp_name(void *dirp)
> +{
> +	switch (tst_variant) {
> +	case 0:
> +		return ((struct linux_dirent *)dirp)->d_name;
> +	case 1:
> +		return ((struct linux_dirent64 *)dirp)->d_name;
> +	case 2:
> +#if HAVE_GETDENTS
> +		return ((struct dirent *)dirp)->d_name;
>  #else
> +		tst_brk(TCONF, "libc getdents() is not implemented");
> +		break;
> +#endif
> +	case 3:
> +#if HAVE_GETDENTS64
> +		return ((struct dirent64 *)dirp)->d_name;
> +#else
> +		tst_brk(TCONF, "libc getdents64() is not implemented");
> +		break;
> +#endif
> +	}
> +	return NULL;
> +}
> +
> +static inline size_t
> +tst_dirp_reclen(void *dirp)
> +{
> +	switch (tst_variant) {
> +	case 0:
> +		return ((struct linux_dirent *)dirp)->d_reclen;
> +	case 1:
> +		return ((struct linux_dirent64 *)dirp)->d_reclen;
> +	case 2:
> +#if HAVE_GETDENTS
> +		return ((struct dirent *)dirp)->d_reclen;
> +#else
> +		tst_brk(TCONF, "libc getdents() is not implemented");
> +		break;
> +#endif
> +	case 3:
> +#if HAVE_GETDENTS64
> +		return ((struct dirent64 *)dirp)->d_reclen;
> +#else
> +		tst_brk(TCONF, "libc getdents64() is not implemented");
> +		break;
> +#endif
> +	}
> +	return 0;
> +}
> +
>  static inline int
> -getdents64(unsigned int fd, struct linux_dirent64 *dirp64, unsigned int size)
> +tst_getdents(int fd, void *dirp, unsigned int size)
> +{
> +	switch (tst_variant) {
> +	case 0:
> +		return linux_getdents(fd, dirp, size);
> +	case 1:
> +		return linux_getdents64(fd, dirp, size);
> +	case 2:
> +#if HAVE_GETDENTS
> +		return getdents(fd, dirp, size);
> +#else
> +		tst_brk(TCONF, "libc getdents() is not implemented");
> +		break;
> +#endif
> +	case 3:
> +#if HAVE_GETDENTS64
> +		return getdents64(fd, dirp, size);
> +#else
> +		tst_brk(TCONF, "libc getdents64() is not implemented");
> +		break;
> +#endif
> +	}
> +	return -1;
> +}
> +
> +static inline void
> +getdents_info(void)
>  {
> -	return ltp_syscall(__NR_getdents64, fd, dirp64, size);
> +	switch (tst_variant) {
> +	case 0:
> +		tst_res(TINFO, "Testing the SYS_getdents syscall");
> +		break;
> +	case 1:
> +		tst_res(TINFO, "Testing the SYS_getdents64 syscall");
> +		break;
> +	case 2:
> +#if HAVE_GETDENTS
> +		tst_res(TINFO, "Testing libc getdents()");
> +#else
> +		tst_brk(TCONF, "libc getdents() is not implemented");
> +#endif
> +		break;
> +	case 3:
> +#if HAVE_GETDENTS64
> +		tst_res(TINFO, "Testing libc getdents64()");
> +#else
> +		tst_brk(TCONF, "libc getdents64() is not implemented");
> +#endif
> +		break;
> +	}
>  }
> -#endif /* HAVE_GETDENTS64 */
> +
> +#define TEST_VARIANTS 4
> +
>  #endif /* GETDENTS_H */
> diff --git a/testcases/kernel/syscalls/getdents/getdents01.c b/testcases/kernel/syscalls/getdents/getdents01.c
> index eca572d42..bdf939ab0 100644
> --- a/testcases/kernel/syscalls/getdents/getdents01.c
> +++ b/testcases/kernel/syscalls/getdents/getdents01.c
> @@ -20,16 +20,12 @@
>   */
>  
>  #define _GNU_SOURCE
> -#include <stdio.h>
> -#include <errno.h>
> -#include <sys/types.h>
> -#include <sys/stat.h>
> -#include <fcntl.h>
> -
> -#include "test.h"
> -#include "safe_macros.h"
> +
> +#include "tst_test.h"
>  #include "getdents.h"
>  
> +#include <stdlib.h>
> +
>  static void cleanup(void);
>  static void setup(void);

These two declarations are useless now.

> @@ -37,21 +33,7 @@ static void reset_flags(void);
>  static void check_flags(void);
>  static void set_flag(const char *name);
>  
> -char *TCID = "getdents01";
> -int TST_TOTAL = 1;
> -
> -static int longsyscall;
> -
> -static option_t options[] = {
> -		/* -l long option. Tests getdents64 */
> -		{"l", &longsyscall, NULL},
> -		{NULL, NULL, NULL}
> -};
> -
> -static void help(void)
> -{
> -	printf("  -l      Test the getdents64 system call\n");
> -}
> +static int fd;
>  
>  enum entry_type {
>  	ENTRY_DIR,
> @@ -80,95 +62,49 @@ struct testcase testcases[] = {
>   */
>  #define BUFSIZE 512
>  
> -int main(int ac, char **av)
> +void run(void)
>  {
> -	int lc;
> -	int rval, fd;
> -	struct linux_dirent64 *dirp64;
> -	struct linux_dirent *dirp;
> -	void *buf;
> -
> -	tst_parse_opts(ac, av, options, &help);
> +	int rval;
> +	void *dirp;
>  
>  	/* The buffer is allocated to make sure it's suitably aligned */
> -	buf = malloc(BUFSIZE);
> -
> -	if (buf == NULL)
> -		tst_brkm(TBROK, NULL, "malloc failed");
> -
> -	dirp64 = buf;
> -	dirp = buf;
> +	dirp = tst_alloc(BUFSIZE);
>  
> -	setup();
> +	if (dirp == NULL)
> +		tst_brk(TBROK, "malloc failed");

This is not how tst_alloc() is supposed to work.

First of all it never fails, but that is not the most important problem
here. The guarded buffers are supposed to be allocated in the test setup
once for the lifetime of the test.

The run() function could be looped over (with the -i command line
parameter) which would trigger allocation failure sooner or later.

Constant size buffers can be allocated by setting the .bufs in the
tst_test structure, see add_keys01.c for example.

> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -		const char *d_name;
> +	rval = tst_getdents(fd, dirp, BUFSIZE);
>  
> -		tst_count = 0;
> -
> -		if ((fd = open(".", O_RDONLY)) == -1)
> -			tst_brkm(TBROK, cleanup, "open of directory failed");
> -
> -		if (longsyscall)
> -			rval = getdents64(fd, dirp64, BUFSIZE);
> +	if (rval < 0) {
> +		if (errno == ENOSYS)
> +			tst_brk(TCONF, "syscall not implemented");
>  		else
> -			rval = getdents(fd, dirp, BUFSIZE);
> -
> -		if (rval < 0) {
> -			if (errno == ENOSYS)
> -				tst_resm(TCONF, "syscall not implemented");
> -			else
> -				tst_resm(TFAIL | TERRNO,
> -				         "getdents failed unexpectedly");
> -			continue;
> -		}
> -
> -		if (rval == 0) {
> -			tst_resm(TFAIL,
> -				 "getdents failed - returned end of directory");
> -			continue;
> -		}
> -
> -		reset_flags();
> -
> -		do {
> -			size_t d_reclen;
> +			tst_brk(TFAIL | TERRNO, "getdents failed unexpectedly");
> +	}
>  
> -			if (longsyscall) {
> -				d_reclen = dirp64->d_reclen;
> -				d_name = dirp64->d_name;
> -			} else {
> -				d_reclen = dirp->d_reclen;
> -				d_name = dirp->d_name;
> -			}
> +	if (rval == 0)
> +		tst_brk(TFAIL, "getdents failed - returned end of directory");

You must not use tst_brk() with TFAIL, here you should use tst_res()
followed by a return.

> -			set_flag(d_name);
> +	reset_flags();
>  
> -			tst_resm(TINFO, "Found '%s'", d_name);
> -			
> -			rval -= d_reclen;
> -			
> -			if (longsyscall)
> -				dirp64 = (void*)dirp64 + d_reclen;
> -			else
> -				dirp = (void*)dirp + d_reclen;
> +	do {
> +		size_t d_reclen = tst_dirp_reclen(dirp);
> +		const char *d_name = tst_dirp_name(dirp);
>  
> -		} while (rval > 0);
> +		set_flag(d_name);
>  
> -		SAFE_CLOSE(cleanup, fd);
> -	
> -		check_flags();
> -	}
> +		tst_res(TINFO, "Found '%s'", d_name);
>  
> -	free(buf);
> +		rval -= d_reclen;
> +		dirp += d_reclen;
> +	} while (rval > 0);
>  
> -	cleanup();
> -	tst_exit();
> +	check_flags();
>  }
>  
>  static void reset_flags(void)
>  {
> -	int i;
> +	size_t i;
>  
>  	for (i = 0; i < ARRAY_SIZE(testcases); i++)
>  		testcases[i].found = 0;
> @@ -176,24 +112,25 @@ static void reset_flags(void)
>  
>  static void check_flags(void)
>  {
> -	int i, err = 0;
> +	size_t i;
> +	int err = 0;
>  
>  	for (i = 0; i < ARRAY_SIZE(testcases); i++) {
>  		if (!testcases[i].found) {
> -			tst_resm(TINFO, "Entry '%s' not found", testcases[i].name);
> +			tst_res(TINFO, "Entry '%s' not found", testcases[i].name);
>  			err++;
>  		}
>  	}
>  
>  	if (err)
> -		tst_resm(TFAIL, "Some entires not found");
> +		tst_res(TFAIL, "Some entries not found");
>  	else
> -		tst_resm(TPASS, "All entires found");
> +		tst_res(TPASS, "All entries found");
>  }
>  
>  static void set_flag(const char *name)
>  {
> -	int i;
> +	size_t i;
>  
>  	for (i = 0; i < ARRAY_SIZE(testcases); i++) {
>  		if (!strcmp(name, testcases[i].name)) {
> @@ -202,39 +139,47 @@ static void set_flag(const char *name)
>  		}
>  	}
>  
> -	tst_resm(TFAIL, "Unexpected entry '%s' found", name);
> +	tst_res(TFAIL, "Unexpected entry '%s' found", name);
>  }
>  
>  static void setup(void)
>  {
> -	int i;
> -
> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> -
> -	tst_tmpdir();
> -
> -	for (i = 0; i < ARRAY_SIZE(testcases); i++) {
> -		
> -		if (!testcases[i].create)
> -			continue;
> -	
> -		switch (testcases[i].type) {
> -		case ENTRY_DIR:
> -			SAFE_MKDIR(cleanup, testcases[i].name, 0777);
> -		break;
> -		case ENTRY_FILE:
> -			SAFE_FILE_PRINTF(cleanup, testcases[i].name, " ");
> -		break;
> -		case ENTRY_SYMLINK:
> -			SAFE_SYMLINK(cleanup, "nonexistent", testcases[i].name);
> -		break;
> +	size_t i;
> +
> +	getdents_info();
> +
> +	if (!tst_variant) {
> +		for (i = 0; i < ARRAY_SIZE(testcases); i++) {
> +			if (!testcases[i].create)
> +				continue;
> +
> +			switch (testcases[i].type) {
> +			case ENTRY_DIR:
> +				SAFE_MKDIR(testcases[i].name, 0777);
> +			break;
> +			case ENTRY_FILE:
> +				SAFE_FILE_PRINTF(testcases[i].name, " ");
> +			break;
> +			case ENTRY_SYMLINK:
> +				SAFE_SYMLINK("nonexistent", testcases[i].name);
> +			break;
> +			}
>  		}
>  	}
>  
> -	TEST_PAUSE;
> +	fd = SAFE_OPEN(".", O_RDONLY|O_DIRECTORY);
>  }
>  
>  static void cleanup(void)
>  {
> -	tst_rmdir();
> +	if (fd != 0)
> +		SAFE_CLOSE(fd);
>  }
> +
> +static struct tst_test test = {
> +	.needs_tmpdir = 1,
> +	.test_all = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_variants = TEST_VARIANTS,
> +};
> diff --git a/testcases/kernel/syscalls/getdents/getdents02.c b/testcases/kernel/syscalls/getdents/getdents02.c
> index 4cf54aea3..82932b7b1 100644
> --- a/testcases/kernel/syscalls/getdents/getdents02.c
> +++ b/testcases/kernel/syscalls/getdents/getdents02.c
> @@ -34,22 +34,15 @@
>   */
>  
>  #define _GNU_SOURCE
> -#include <stdio.h>
>  #include <errno.h>
> -#include <sys/types.h>
> -#include <sys/stat.h>
> -#include <fcntl.h>
>  
> -#include "test.h"
> +#include "tst_test.h"
>  #include "getdents.h"
> -#include "safe_macros.h"
>  
>  #define DIR_MODE	(S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP| \
>  			 S_IXGRP|S_IROTH|S_IXOTH)
>  #define TEST_DIR	"test_dir"
>  
> -static void cleanup(void);
> -static void setup(void);
>  static void print_test_result(int err, int exp_errno);
>  
>  char *TCID = "getdents02";
> @@ -64,58 +57,33 @@ static void (*testfunc[])(void) = { test_ebadf, test_einval,
>  
>  int TST_TOTAL = ARRAY_SIZE(testfunc);
>  
> -static int longsyscall;
> +static void *dirp;
> +static size_t size;
>  
> -option_t options[] = {
> -		/* -l long option. Tests getdents64 */
> -		{"l", &longsyscall, NULL},
> -		{NULL, NULL, NULL}
> -};
> -
> -static void help(void)
> -{
> -	printf("  -l      Test the getdents64 system call\n");
> -}
> -
> -int main(int ac, char **av)
> +static void setup(void)
>  {
> -	int lc, i;
> -
> -	tst_parse_opts(ac, av, options, &help);
> -
> -	setup();
> -
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -		tst_count = 0;
> +	getdents_info();
>  
> -		for (i = 0; i < TST_TOTAL; i++)
> -			(*testfunc[i])();
> -	}
> -
> -	cleanup();
> -	tst_exit();
> +	dirp = tst_dirp_alloc();
> +	size = tst_dirp_size();
>  }
>  
> -static void setup(void)
> +static void run(unsigned int i)
>  {
> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> -
> -	tst_tmpdir();
> -
> -	TEST_PAUSE;
> +	testfunc[i]();
>  }
>  
>  static void print_test_result(int err, int exp_errno)
>  {
>  	if (err == 0) {
> -		tst_resm(TFAIL, "call succeeded unexpectedly");
> +		tst_res(TFAIL, "call succeeded unexpectedly");
>  	} else if  (err == exp_errno) {
> -		tst_resm(TPASS, "getdents failed as expected: %s",
> +		tst_res(TPASS, "getdents failed as expected: %s",
>  			 strerror(err));
>  	} else if (err == ENOSYS) {
> -		tst_resm(TCONF, "syscall not implemented");
> +		tst_res(TCONF, "syscall not implemented");
>  	} else {
> -		tst_resm(TFAIL, "getdents failed unexpectedly: %s",
> +		tst_res(TFAIL, "getdents failed unexpectedly: %s",
>  			 strerror(err));
>  	}
>  }
> @@ -123,13 +91,8 @@ static void print_test_result(int err, int exp_errno)
>  static void test_ebadf(void)
>  {
>  	int fd = -5;
> -	struct linux_dirent64 dirp64;
> -	struct linux_dirent dirp;
>  
> -	if (longsyscall)
> -		getdents64(fd, &dirp64, sizeof(dirp64));
> -	else
> -		getdents(fd, &dirp, sizeof(dirp));
> +	tst_getdents(fd, dirp, size);
>  
>  	print_test_result(errno, EBADF);
>  }
> @@ -139,59 +102,49 @@ static void test_einval(void)
>  	int fd;
>  	char buf[1];
>  
> -	fd = SAFE_OPEN(cleanup, ".", O_RDONLY);
> +	fd = SAFE_OPEN(".", O_RDONLY);
>  
>  	/* Pass one byte long buffer. The result should be EINVAL */
> -	if (longsyscall)
> -		getdents64(fd, (void *)buf, sizeof(buf));
> -	else
> -		getdents(fd, (void *)buf, sizeof(buf));
> +	tst_getdents(fd, (void *)buf, sizeof(buf));
>  
>  	print_test_result(errno, EINVAL);
>  
> -	SAFE_CLOSE(cleanup, fd);
> +	SAFE_CLOSE(fd);
>  }
>  
>  static void test_enotdir(void)
>  {
>  	int fd;
> -	struct linux_dirent64 dir64;
> -	struct linux_dirent dir;
>  
> -	fd = SAFE_OPEN(cleanup, "test", O_CREAT | O_RDWR, 0644);
> +	fd = SAFE_OPEN("test", O_CREAT | O_RDWR, 0644);
>  
> -	if (longsyscall)
> -		getdents64(fd, &dir64, sizeof(dir64));
> -	else
> -		getdents(fd, &dir, sizeof(dir));
> +	tst_getdents(fd, dirp, size);
>  
>  	print_test_result(errno, ENOTDIR);
>  
> -	SAFE_CLOSE(cleanup, fd);
> +	SAFE_CLOSE(fd);
>  }
>  
>  static void test_enoent(void)
>  {
>  	int fd;
> -	struct linux_dirent64 dir64;
> -	struct linux_dirent dir;
>  
> -	SAFE_MKDIR(cleanup, TEST_DIR, DIR_MODE);
> +	SAFE_MKDIR(TEST_DIR, DIR_MODE);
>  
> -	fd = SAFE_OPEN(cleanup, TEST_DIR, O_DIRECTORY);
> -	SAFE_RMDIR(cleanup, TEST_DIR);
> +	fd = SAFE_OPEN(TEST_DIR, O_DIRECTORY);
> +	SAFE_RMDIR(TEST_DIR);
>  
> -	if (longsyscall)
> -		getdents64(fd, &dir64, sizeof(dir64));
> -	else
> -		getdents(fd, &dir, sizeof(dir));
> +	tst_getdents(fd, dirp, size);
>  
>  	print_test_result(errno, ENOENT);
>  
> -	SAFE_CLOSE(cleanup, fd);
> +	SAFE_CLOSE(fd);
>  }
>  
> -static void cleanup(void)
> -{
> -	tst_rmdir();
> -}
> +static struct tst_test test = {
> +	.needs_tmpdir = 1,
> +	.test = run,
> +	.setup = setup,
> +	.tcnt = ARRAY_SIZE(testfunc),
> +	.test_variants = TEST_VARIANTS,
> +};

Not that it's wrong however these days we tend to write these tests in a
more data driver way such as access/access04.c

Also can you please switch to SPDX licence identifier while you are
touching the source code?



Other than this it looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [LTP] [PATCH v3] getdents: update to the new api, don't mix libc and kernel types
  2020-11-19  8:44 ` [LTP] [PATCH v2] getdents: update to new api, " j.nixdorf
  2020-11-19 13:00   ` Cyril Hrubis
@ 2020-11-20 11:56   ` j.nixdorf
  2020-11-20 13:04     ` Cyril Hrubis
  1 sibling, 1 reply; 6+ messages in thread
From: j.nixdorf @ 2020-11-20 11:56 UTC (permalink / raw)
  To: ltp

Before this commit the tests called libc's getdents(64) with the kernel
types struct linux_dirent(64) as arguments, mixing libc functions and
kernel types.

This breaks on musl libc, as it provides getdents and struct dirent as
an alias to getdents64 and struct dirent64.

To not lose coverage we now test both supported configurations: the
syscall with the kernel types and the libc wrappers with the libc types.

To take advantage of support for multiple test_variants the tests were
converted to the new test API at the same time.

Signed-off-by: Johannes Nixdorf <j.nixdorf@avm.de>

---

v2:
  - Converted to the new test API, using test_variants for all libc and
    syscall variants.

v3:
  - Added SPDX indentifers.
  - Removed superflous calls to test_brk(TCONF, ...) after
    getdents_info from getdents.h.
  - Removed tst_dirp_alloc from getdents.h.
  - Removed useless forward declarations from getdents01.c.
  - Allocate the buffer for getdents01.c through the field bufs on the
    test description object.
  - Rewrite getdents02.c in a data-driven way.

 runtest/syscalls                              |   3 -
 testcases/kernel/syscalls/getdents/getdents.h | 136 ++++++++++--
 .../kernel/syscalls/getdents/getdents01.c     | 202 +++++++-----------
 .../kernel/syscalls/getdents/getdents02.c     | 187 +++++-----------
 travis/alpine.sh                              |   2 -
 5 files changed, 247 insertions(+), 283 deletions(-)

diff --git a/runtest/syscalls b/runtest/syscalls
index 0443f9f3d..8f2fe0db2 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -410,9 +410,6 @@ getcwd04 getcwd04
 getdents01 getdents01
 getdents02 getdents02
 
-getdents01_64 getdents01 -l
-getdents02_64 getdents02 -l
-
 getdomainname01 getdomainname01
 
 getdtablesize01 getdtablesize01
diff --git a/testcases/kernel/syscalls/getdents/getdents.h b/testcases/kernel/syscalls/getdents/getdents.h
index a20b1811a..560df4126 100644
--- a/testcases/kernel/syscalls/getdents/getdents.h
+++ b/testcases/kernel/syscalls/getdents/getdents.h
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) International Business Machines  Corp., 2001
  * Copyright (c) 2013 Cyril Hrubis <chrubis@suse.cz>
@@ -21,9 +22,13 @@
 #define GETDENTS_H
 
 #include <stdint.h>
-#include "test.h"
-#include "lapi/syscalls.h"
 #include "config.h"
+#include "lapi/syscalls.h"
+
+#if HAVE_GETDENTS || HAVE_GETDENTS64
+#include <unistd.h>
+#endif
+
 /*
  * See fs/compat.c struct compat_linux_dirent
  */
@@ -34,17 +39,12 @@ struct linux_dirent {
 	char            d_name[];
 };
 
-#if HAVE_GETDENTS
-#include <unistd.h>
-#else
 static inline int
-getdents(unsigned int fd, struct linux_dirent *dirp, unsigned int size)
+linux_getdents(unsigned int fd, struct linux_dirent *dirp, unsigned int size)
 {
-	return ltp_syscall(__NR_getdents, fd, dirp, size);
+	return tst_syscall(__NR_getdents, fd, dirp, size);
 }
 
-#endif /* HAVE_GETDENTS */
-
 struct linux_dirent64 {
 	uint64_t	d_ino;
 	int64_t		d_off;
@@ -53,14 +53,120 @@ struct linux_dirent64 {
 	char		d_name[];
 };
 
+static inline int
+linux_getdents64(unsigned int fd, struct linux_dirent64 *dirp64, unsigned int size)
+{
+	return tst_syscall(__NR_getdents64, fd, dirp64, size);
+}
+
+static inline size_t
+tst_dirp_size(void)
+{
+	switch (tst_variant) {
+	case 0:
+		return sizeof(struct linux_dirent);
+	case 1:
+		return sizeof(struct linux_dirent64);
+#if HAVE_GETDENTS
+	case 2:
+		return sizeof(struct dirent);
+#endif
 #if HAVE_GETDENTS64
-#include <dirent.h>
-#include <unistd.h>
-#else
+	case 3:
+		return sizeof(struct dirent64);
+#endif
+	}
+	return 0;
+}
+
+static inline const char *
+tst_dirp_name(void *dirp)
+{
+	switch (tst_variant) {
+	case 0:
+		return ((struct linux_dirent *)dirp)->d_name;
+	case 1:
+		return ((struct linux_dirent64 *)dirp)->d_name;
+#if HAVE_GETDENTS
+	case 2:
+		return ((struct dirent *)dirp)->d_name;
+#endif
+#if HAVE_GETDENTS64
+	case 3:
+		return ((struct dirent64 *)dirp)->d_name;
+#endif
+	}
+	return NULL;
+}
+
+static inline size_t
+tst_dirp_reclen(void *dirp)
+{
+	switch (tst_variant) {
+	case 0:
+		return ((struct linux_dirent *)dirp)->d_reclen;
+	case 1:
+		return ((struct linux_dirent64 *)dirp)->d_reclen;
+#if HAVE_GETDENTS
+	case 2:
+		return ((struct dirent *)dirp)->d_reclen;
+#endif
+#if HAVE_GETDENTS64
+	case 3:
+		return ((struct dirent64 *)dirp)->d_reclen;
+#endif
+
+	}
+	return 0;
+}
+
 static inline int
-getdents64(unsigned int fd, struct linux_dirent64 *dirp64, unsigned int size)
+tst_getdents(int fd, void *dirp, unsigned int size)
+{
+	switch (tst_variant) {
+	case 0:
+		return linux_getdents(fd, dirp, size);
+	case 1:
+		return linux_getdents64(fd, dirp, size);
+#if HAVE_GETDENTS
+	case 2:
+		return getdents(fd, dirp, size);
+#endif
+#if HAVE_GETDENTS64
+	case 3:
+		return getdents64(fd, dirp, size);
+#endif
+	}
+	return -1;
+}
+
+static inline void
+getdents_info(void)
 {
-	return ltp_syscall(__NR_getdents64, fd, dirp64, size);
+	switch (tst_variant) {
+	case 0:
+		tst_res(TINFO, "Testing the SYS_getdents syscall");
+		break;
+	case 1:
+		tst_res(TINFO, "Testing the SYS_getdents64 syscall");
+		break;
+	case 2:
+#if HAVE_GETDENTS
+		tst_res(TINFO, "Testing libc getdents()");
+#else
+		tst_brk(TCONF, "libc getdents() is not implemented");
+#endif
+		break;
+	case 3:
+#if HAVE_GETDENTS64
+		tst_res(TINFO, "Testing libc getdents64()");
+#else
+		tst_brk(TCONF, "libc getdents64() is not implemented");
+#endif
+		break;
+	}
 }
-#endif /* HAVE_GETDENTS64 */
+
+#define TEST_VARIANTS 4
+
 #endif /* GETDENTS_H */
diff --git a/testcases/kernel/syscalls/getdents/getdents01.c b/testcases/kernel/syscalls/getdents/getdents01.c
index eca572d42..81cebf76c 100644
--- a/testcases/kernel/syscalls/getdents/getdents01.c
+++ b/testcases/kernel/syscalls/getdents/getdents01.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) International Business Machines  Corp., 2001
  *	         written by Wayne Boyer
@@ -20,38 +21,17 @@
  */
 
 #define _GNU_SOURCE
-#include <stdio.h>
-#include <errno.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-
-#include "test.h"
-#include "safe_macros.h"
+
+#include "tst_test.h"
 #include "getdents.h"
 
-static void cleanup(void);
-static void setup(void);
+#include <stdlib.h>
 
 static void reset_flags(void);
 static void check_flags(void);
 static void set_flag(const char *name);
 
-char *TCID = "getdents01";
-int TST_TOTAL = 1;
-
-static int longsyscall;
-
-static option_t options[] = {
-		/* -l long option. Tests getdents64 */
-		{"l", &longsyscall, NULL},
-		{NULL, NULL, NULL}
-};
-
-static void help(void)
-{
-	printf("  -l      Test the getdents64 system call\n");
-}
+static int fd;
 
 enum entry_type {
 	ENTRY_DIR,
@@ -80,95 +60,48 @@ struct testcase testcases[] = {
  */
 #define BUFSIZE 512
 
-int main(int ac, char **av)
-{
-	int lc;
-	int rval, fd;
-	struct linux_dirent64 *dirp64;
-	struct linux_dirent *dirp;
-	void *buf;
-
-	tst_parse_opts(ac, av, options, &help);
-
-	/* The buffer is allocated to make sure it's suitably aligned */
-	buf = malloc(BUFSIZE);
-
-	if (buf == NULL)
-		tst_brkm(TBROK, NULL, "malloc failed");
-
-	dirp64 = buf;
-	dirp = buf;
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		const char *d_name;
-
-		tst_count = 0;
-
-		if ((fd = open(".", O_RDONLY)) == -1)
-			tst_brkm(TBROK, cleanup, "open of directory failed");
+static void *dirp;
 
-		if (longsyscall)
-			rval = getdents64(fd, dirp64, BUFSIZE);
-		else
-			rval = getdents(fd, dirp, BUFSIZE);
+static void run(void)
+{
+	int rval;
 
-		if (rval < 0) {
-			if (errno == ENOSYS)
-				tst_resm(TCONF, "syscall not implemented");
-			else
-				tst_resm(TFAIL | TERRNO,
-				         "getdents failed unexpectedly");
-			continue;
-		}
+	rval = tst_getdents(fd, dirp, BUFSIZE);
 
-		if (rval == 0) {
-			tst_resm(TFAIL,
-				 "getdents failed - returned end of directory");
-			continue;
+	if (rval < 0) {
+		if (errno == ENOSYS)
+			tst_brk(TCONF, "syscall not implemented");
+		else {
+			tst_res(TFAIL | TERRNO, "getdents failed unexpectedly");
+			return;
 		}
+	}
 
-		reset_flags();
-
-		do {
-			size_t d_reclen;
-
-			if (longsyscall) {
-				d_reclen = dirp64->d_reclen;
-				d_name = dirp64->d_name;
-			} else {
-				d_reclen = dirp->d_reclen;
-				d_name = dirp->d_name;
-			}
+	if (rval == 0) {
+		tst_res(TFAIL, "getdents failed - returned end of directory");
+		return;
+	}
 
-			set_flag(d_name);
+	reset_flags();
 
-			tst_resm(TINFO, "Found '%s'", d_name);
-			
-			rval -= d_reclen;
-			
-			if (longsyscall)
-				dirp64 = (void*)dirp64 + d_reclen;
-			else
-				dirp = (void*)dirp + d_reclen;
+	do {
+		size_t d_reclen = tst_dirp_reclen(dirp);
+		const char *d_name = tst_dirp_name(dirp);
 
-		} while (rval > 0);
+		set_flag(d_name);
 
-		SAFE_CLOSE(cleanup, fd);
-	
-		check_flags();
-	}
+		tst_res(TINFO, "Found '%s'", d_name);
 
-	free(buf);
+		rval -= d_reclen;
+		dirp += d_reclen;
+	} while (rval > 0);
 
-	cleanup();
-	tst_exit();
+	check_flags();
 }
 
 static void reset_flags(void)
 {
-	int i;
+	size_t i;
 
 	for (i = 0; i < ARRAY_SIZE(testcases); i++)
 		testcases[i].found = 0;
@@ -176,24 +109,25 @@ static void reset_flags(void)
 
 static void check_flags(void)
 {
-	int i, err = 0;
+	size_t i;
+	int err = 0;
 
 	for (i = 0; i < ARRAY_SIZE(testcases); i++) {
 		if (!testcases[i].found) {
-			tst_resm(TINFO, "Entry '%s' not found", testcases[i].name);
+			tst_res(TINFO, "Entry '%s' not found", testcases[i].name);
 			err++;
 		}
 	}
 
 	if (err)
-		tst_resm(TFAIL, "Some entires not found");
+		tst_res(TFAIL, "Some entries not found");
 	else
-		tst_resm(TPASS, "All entires found");
+		tst_res(TPASS, "All entries found");
 }
 
 static void set_flag(const char *name)
 {
-	int i;
+	size_t i;
 
 	for (i = 0; i < ARRAY_SIZE(testcases); i++) {
 		if (!strcmp(name, testcases[i].name)) {
@@ -202,39 +136,51 @@ static void set_flag(const char *name)
 		}
 	}
 
-	tst_resm(TFAIL, "Unexpected entry '%s' found", name);
+	tst_res(TFAIL, "Unexpected entry '%s' found", name);
 }
 
 static void setup(void)
 {
-	int i;
-
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
-	tst_tmpdir();
-
-	for (i = 0; i < ARRAY_SIZE(testcases); i++) {
-		
-		if (!testcases[i].create)
-			continue;
-	
-		switch (testcases[i].type) {
-		case ENTRY_DIR:
-			SAFE_MKDIR(cleanup, testcases[i].name, 0777);
-		break;
-		case ENTRY_FILE:
-			SAFE_FILE_PRINTF(cleanup, testcases[i].name, " ");
-		break;
-		case ENTRY_SYMLINK:
-			SAFE_SYMLINK(cleanup, "nonexistent", testcases[i].name);
-		break;
+	size_t i;
+
+	getdents_info();
+
+	if (!tst_variant) {
+		for (i = 0; i < ARRAY_SIZE(testcases); i++) {
+			if (!testcases[i].create)
+				continue;
+
+			switch (testcases[i].type) {
+			case ENTRY_DIR:
+				SAFE_MKDIR(testcases[i].name, 0777);
+			break;
+			case ENTRY_FILE:
+				SAFE_FILE_PRINTF(testcases[i].name, " ");
+			break;
+			case ENTRY_SYMLINK:
+				SAFE_SYMLINK("nonexistent", testcases[i].name);
+			break;
+			}
 		}
 	}
 
-	TEST_PAUSE;
+	fd = SAFE_OPEN(".", O_RDONLY|O_DIRECTORY);
 }
 
 static void cleanup(void)
 {
-	tst_rmdir();
+	if (fd != 0)
+		SAFE_CLOSE(fd);
 }
+
+static struct tst_test test = {
+	.needs_tmpdir = 1,
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.bufs = (struct tst_buffers []) {
+		{&dirp, .size = BUFSIZE},
+		{},
+	},
+	.test_variants = TEST_VARIANTS,
+};
diff --git a/testcases/kernel/syscalls/getdents/getdents02.c b/testcases/kernel/syscalls/getdents/getdents02.c
index 4cf54aea3..42691f5f0 100644
--- a/testcases/kernel/syscalls/getdents/getdents02.c
+++ b/testcases/kernel/syscalls/getdents/getdents02.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) International Business Machines  Corp., 2001
  *	         written by Wayne Boyer
@@ -34,164 +35,80 @@
  */
 
 #define _GNU_SOURCE
-#include <stdio.h>
 #include <errno.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
 
-#include "test.h"
+#include "tst_test.h"
 #include "getdents.h"
-#include "safe_macros.h"
 
 #define DIR_MODE	(S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP| \
 			 S_IXGRP|S_IROTH|S_IXOTH)
 #define TEST_DIR	"test_dir"
 
-static void cleanup(void);
-static void setup(void);
-static void print_test_result(int err, int exp_errno);
-
 char *TCID = "getdents02";
 
-static void test_ebadf(void);
-static void test_einval(void);
-static void test_enotdir(void);
-static void test_enoent(void);
-
-static void (*testfunc[])(void) = { test_ebadf, test_einval,
-				    test_enotdir, test_enoent };
-
-int TST_TOTAL = ARRAY_SIZE(testfunc);
-
-static int longsyscall;
-
-option_t options[] = {
-		/* -l long option. Tests getdents64 */
-		{"l", &longsyscall, NULL},
-		{NULL, NULL, NULL}
+static char *dirp;
+static size_t size;
+
+static char dirp1_arr[1];
+static char *dirp1 = dirp1_arr;
+static size_t size1 = 1;
+
+static int fd_inv = -5;
+static int fd;
+static int fd_file;
+static int fd_unlinked;
+
+static struct tcase {
+	int *fd;
+	char **dirp;
+	size_t *size;
+	int exp_errno;
+} tcases[] = {
+	{ &fd_inv, &dirp, &size, EBADF },
+	{ &fd, &dirp1, &size1, EINVAL },
+	{ &fd_file, &dirp, &size, ENOTDIR },
+	{ &fd_unlinked, &dirp, &size, ENOENT },
 };
 
-static void help(void)
-{
-	printf("  -l      Test the getdents64 system call\n");
-}
-
-int main(int ac, char **av)
+static void setup(void)
 {
-	int lc, i;
+	getdents_info();
 
-	tst_parse_opts(ac, av, options, &help);
+	size = tst_dirp_size();
+	dirp = tst_alloc(size);
 
-	setup();
+	fd = SAFE_OPEN(".", O_RDONLY);
+	fd_file = SAFE_OPEN("test", O_CREAT | O_RDWR, 0644);
 
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
+	SAFE_MKDIR(TEST_DIR, DIR_MODE);
+	fd_unlinked = SAFE_OPEN(TEST_DIR, O_DIRECTORY);
+	SAFE_RMDIR(TEST_DIR);
 
-		for (i = 0; i < TST_TOTAL; i++)
-			(*testfunc[i])();
-	}
-
-	cleanup();
-	tst_exit();
 }
 
-static void setup(void)
+static void run(unsigned int i)
 {
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
+	struct tcase *tc = tcases + i;
 
-	tst_tmpdir();
+	tst_getdents(*tc->fd, *tc->dirp, *tc->size);
 
-	TEST_PAUSE;
-}
-
-static void print_test_result(int err, int exp_errno)
-{
-	if (err == 0) {
-		tst_resm(TFAIL, "call succeeded unexpectedly");
-	} else if  (err == exp_errno) {
-		tst_resm(TPASS, "getdents failed as expected: %s",
-			 strerror(err));
-	} else if (err == ENOSYS) {
-		tst_resm(TCONF, "syscall not implemented");
+	if (errno == 0) {
+		tst_res(TFAIL, "call succeeded unexpectedly");
+	} else if (errno == tc->exp_errno) {
+		tst_res(TPASS, "getdents failed as expected: %s",
+			 strerror(errno));
+	} else if (errno == ENOSYS) {
+		tst_res(TCONF, "syscall not implemented");
 	} else {
-		tst_resm(TFAIL, "getdents failed unexpectedly: %s",
-			 strerror(err));
+		tst_res(TFAIL, "getdents failed unexpectedly: %s",
+			 strerror(errno));
 	}
 }
 
-static void test_ebadf(void)
-{
-	int fd = -5;
-	struct linux_dirent64 dirp64;
-	struct linux_dirent dirp;
-
-	if (longsyscall)
-		getdents64(fd, &dirp64, sizeof(dirp64));
-	else
-		getdents(fd, &dirp, sizeof(dirp));
-
-	print_test_result(errno, EBADF);
-}
-
-static void test_einval(void)
-{
-	int fd;
-	char buf[1];
-
-	fd = SAFE_OPEN(cleanup, ".", O_RDONLY);
-
-	/* Pass one byte long buffer. The result should be EINVAL */
-	if (longsyscall)
-		getdents64(fd, (void *)buf, sizeof(buf));
-	else
-		getdents(fd, (void *)buf, sizeof(buf));
-
-	print_test_result(errno, EINVAL);
-
-	SAFE_CLOSE(cleanup, fd);
-}
-
-static void test_enotdir(void)
-{
-	int fd;
-	struct linux_dirent64 dir64;
-	struct linux_dirent dir;
-
-	fd = SAFE_OPEN(cleanup, "test", O_CREAT | O_RDWR, 0644);
-
-	if (longsyscall)
-		getdents64(fd, &dir64, sizeof(dir64));
-	else
-		getdents(fd, &dir, sizeof(dir));
-
-	print_test_result(errno, ENOTDIR);
-
-	SAFE_CLOSE(cleanup, fd);
-}
-
-static void test_enoent(void)
-{
-	int fd;
-	struct linux_dirent64 dir64;
-	struct linux_dirent dir;
-
-	SAFE_MKDIR(cleanup, TEST_DIR, DIR_MODE);
-
-	fd = SAFE_OPEN(cleanup, TEST_DIR, O_DIRECTORY);
-	SAFE_RMDIR(cleanup, TEST_DIR);
-
-	if (longsyscall)
-		getdents64(fd, &dir64, sizeof(dir64));
-	else
-		getdents(fd, &dir, sizeof(dir));
-
-	print_test_result(errno, ENOENT);
-
-	SAFE_CLOSE(cleanup, fd);
-}
-
-static void cleanup(void)
-{
-	tst_rmdir();
-}
+static struct tst_test test = {
+	.needs_tmpdir = 1,
+	.test = run,
+	.setup = setup,
+	.tcnt = ARRAY_SIZE(tcases),
+	.test_variants = TEST_VARIANTS,
+};
diff --git a/travis/alpine.sh b/travis/alpine.sh
index 61ef144a8..ffd5a8ebc 100755
--- a/travis/alpine.sh
+++ b/travis/alpine.sh
@@ -33,8 +33,6 @@ rm -rfv \
 	testcases/kernel/syscalls/confstr/confstr01.c \
 	testcases/kernel/syscalls/fmtmsg/fmtmsg01.c \
 	testcases/kernel/syscalls/getcontext/getcontext01.c \
-	testcases/kernel/syscalls/getdents/getdents01.c \
-	testcases/kernel/syscalls/getdents/getdents02.c \
 	testcases/kernel/syscalls/rt_tgsigqueueinfo/rt_tgsigqueueinfo01.c \
 	testcases/kernel/syscalls/timer_create/timer_create01.c \
 	testcases/kernel/syscalls/timer_create/timer_create03.c \
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [LTP] [PATCH v3] getdents: update to the new api, don't mix libc and kernel types
  2020-11-20 11:56   ` [LTP] [PATCH v3] getdents: update to the " j.nixdorf
@ 2020-11-20 13:04     ` Cyril Hrubis
  0 siblings, 0 replies; 6+ messages in thread
From: Cyril Hrubis @ 2020-11-20 13:04 UTC (permalink / raw)
  To: ltp

Hi!
Pushed with a minor changes, thanks.

* Removed the GPL license text since it's replaced by the SPDX identifier

* Formatted the test description into asciidoc and changed the comment
  so that it's picked up by documentation parser

* Checked explictily for -1 return value in negative test

* Made use of TERROR flags instead of usings strerror() to print errnos

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-20 13:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-16 12:43 [LTP] [PATCH] syscalls/getdents: don't mix libc and kernel types j.nixdorf
2020-11-18 13:58 ` Cyril Hrubis
2020-11-19  8:44 ` [LTP] [PATCH v2] getdents: update to new api, " j.nixdorf
2020-11-19 13:00   ` Cyril Hrubis
2020-11-20 11:56   ` [LTP] [PATCH v3] getdents: update to the " j.nixdorf
2020-11-20 13:04     ` Cyril Hrubis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.