diff mbox series

[v2,1/3] selftests: x86: test_vsyscall: reorder code to reduce #ifdef blocks

Message ID 20240327184637.3915969-1-usama.anjum@collabora.com (mailing list archive)
State Accepted
Commit b40ac1100b653e86b47aa664dd11e4c1e32b0aad
Headers show
Series [v2,1/3] selftests: x86: test_vsyscall: reorder code to reduce #ifdef blocks | expand

Commit Message

Muhammad Usama Anjum March 27, 2024, 6:46 p.m. UTC
There are multiple #ifdef blocks inside functions where they return just
0 if #ifdef is false. This makes number of tests counting difficult.
Move those functions inside one #ifdef block and move all of them
together. This is preparatory patch for next patch to convert this into
TAP format. So in this patch, we are just moving functions around
without any changes.

With and without this patch, the output of this patch is same.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 tools/testing/selftests/x86/test_vsyscall.c | 176 +++++++++-----------
 1 file changed, 83 insertions(+), 93 deletions(-)

Comments

Shuah Khan March 28, 2024, 6:39 p.m. UTC | #1
On 3/27/24 12:46, Muhammad Usama Anjum wrote:
> There are multiple #ifdef blocks inside functions where they return just
> 0 if #ifdef is false. This makes number of tests counting difficult.
> Move those functions inside one #ifdef block and move all of them
> together. This is preparatory patch for next patch to convert this into
> TAP format. So in this patch, we are just moving functions around
> without any changes.
> 
> With and without this patch, the output of this patch is same.
> 
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---

Pulled all three patches in this series. In the future, please
add a cover-letter when you send patch series.

thanks,
-- Shuah
diff mbox series

Patch

diff --git a/tools/testing/selftests/x86/test_vsyscall.c b/tools/testing/selftests/x86/test_vsyscall.c
index 47cab972807c4..c78b0648aa524 100644
--- a/tools/testing/selftests/x86/test_vsyscall.c
+++ b/tools/testing/selftests/x86/test_vsyscall.c
@@ -21,6 +21,7 @@ 
 #include <sys/uio.h>
 
 #include "helpers.h"
+#include "../kselftest.h"
 
 #ifdef __x86_64__
 # define VSYS(x) (x)
@@ -39,18 +40,6 @@ 
 /* max length of lines in /proc/self/maps - anything longer is skipped here */
 #define MAPS_LINE_LEN 128
 
-static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *),
-		       int flags)
-{
-	struct sigaction sa;
-	memset(&sa, 0, sizeof(sa));
-	sa.sa_sigaction = handler;
-	sa.sa_flags = SA_SIGINFO | flags;
-	sigemptyset(&sa.sa_mask);
-	if (sigaction(sig, &sa, 0))
-		err(1, "sigaction");
-}
-
 /* vsyscalls and vDSO */
 bool vsyscall_map_r = false, vsyscall_map_x = false;
 
@@ -96,64 +85,6 @@  static void init_vdso(void)
 		printf("[WARN]\tfailed to find getcpu in vDSO\n");
 }
 
-static int init_vsys(void)
-{
-#ifdef __x86_64__
-	int nerrs = 0;
-	FILE *maps;
-	char line[MAPS_LINE_LEN];
-	bool found = false;
-
-	maps = fopen("/proc/self/maps", "r");
-	if (!maps) {
-		printf("[WARN]\tCould not open /proc/self/maps -- assuming vsyscall is r-x\n");
-		vsyscall_map_r = true;
-		return 0;
-	}
-
-	while (fgets(line, MAPS_LINE_LEN, maps)) {
-		char r, x;
-		void *start, *end;
-		char name[MAPS_LINE_LEN];
-
-		/* sscanf() is safe here as strlen(name) >= strlen(line) */
-		if (sscanf(line, "%p-%p %c-%cp %*x %*x:%*x %*u %s",
-			   &start, &end, &r, &x, name) != 5)
-			continue;
-
-		if (strcmp(name, "[vsyscall]"))
-			continue;
-
-		printf("\tvsyscall map: %s", line);
-
-		if (start != (void *)0xffffffffff600000 ||
-		    end != (void *)0xffffffffff601000) {
-			printf("[FAIL]\taddress range is nonsense\n");
-			nerrs++;
-		}
-
-		printf("\tvsyscall permissions are %c-%c\n", r, x);
-		vsyscall_map_r = (r == 'r');
-		vsyscall_map_x = (x == 'x');
-
-		found = true;
-		break;
-	}
-
-	fclose(maps);
-
-	if (!found) {
-		printf("\tno vsyscall map in /proc/self/maps\n");
-		vsyscall_map_r = false;
-		vsyscall_map_x = false;
-	}
-
-	return nerrs;
-#else
-	return 0;
-#endif
-}
-
 /* syscalls */
 static inline long sys_gtod(struct timeval *tv, struct timezone *tz)
 {
@@ -176,17 +107,6 @@  static inline long sys_getcpu(unsigned * cpu, unsigned * node,
 	return syscall(SYS_getcpu, cpu, node, cache);
 }
 
-static jmp_buf jmpbuf;
-static volatile unsigned long segv_err;
-
-static void sigsegv(int sig, siginfo_t *info, void *ctx_void)
-{
-	ucontext_t *ctx = (ucontext_t *)ctx_void;
-
-	segv_err =  ctx->uc_mcontext.gregs[REG_ERR];
-	siglongjmp(jmpbuf, 1);
-}
-
 static double tv_diff(const struct timeval *a, const struct timeval *b)
 {
 	return (double)(a->tv_sec - b->tv_sec) +
@@ -396,9 +316,33 @@  static int test_getcpu(int cpu)
 	return nerrs;
 }
 
+#ifdef __x86_64__
+
+static jmp_buf jmpbuf;
+static volatile unsigned long segv_err;
+
+static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *),
+		       int flags)
+{
+	struct sigaction sa;
+	memset(&sa, 0, sizeof(sa));
+	sa.sa_sigaction = handler;
+	sa.sa_flags = SA_SIGINFO | flags;
+	sigemptyset(&sa.sa_mask);
+	if (sigaction(sig, &sa, 0))
+		err(1, "sigaction");
+}
+
+static void sigsegv(int sig, siginfo_t *info, void *ctx_void)
+{
+	ucontext_t *ctx = (ucontext_t *)ctx_void;
+
+	segv_err =  ctx->uc_mcontext.gregs[REG_ERR];
+	siglongjmp(jmpbuf, 1);
+}
+
 static int test_vsys_r(void)
 {
-#ifdef __x86_64__
 	printf("[RUN]\tChecking read access to the vsyscall page\n");
 	bool can_read;
 	if (sigsetjmp(jmpbuf, 1) == 0) {
@@ -420,14 +364,12 @@  static int test_vsys_r(void)
 		printf("[OK]\tWe do not have read access: #PF(0x%lx)\n",
 		       segv_err);
 	}
-#endif
 
 	return 0;
 }
 
 static int test_vsys_x(void)
 {
-#ifdef __x86_64__
 	if (vsyscall_map_x) {
 		/* We already tested this adequately. */
 		return 0;
@@ -454,8 +396,6 @@  static int test_vsys_x(void)
 		       segv_err);
 		return 1;
 	}
-#endif
-
 	return 0;
 }
 
@@ -472,7 +412,6 @@  static int test_vsys_x(void)
  */
 static int test_process_vm_readv(void)
 {
-#ifdef __x86_64__
 	char buf[4096];
 	struct iovec local, remote;
 	int ret;
@@ -504,12 +443,63 @@  static int test_process_vm_readv(void)
 		printf("[FAIL]\tprocess_rm_readv() succeeded, but it should have failed in this configuration\n");
 		return 1;
 	}
-#endif
-
 	return 0;
 }
 
-#ifdef __x86_64__
+static int init_vsys(void)
+{
+	int nerrs = 0;
+	FILE *maps;
+	char line[MAPS_LINE_LEN];
+	bool found = false;
+
+	maps = fopen("/proc/self/maps", "r");
+	if (!maps) {
+		printf("[WARN]\tCould not open /proc/self/maps -- assuming vsyscall is r-x\n");
+		vsyscall_map_r = true;
+		return 0;
+	}
+
+	while (fgets(line, MAPS_LINE_LEN, maps)) {
+		char r, x;
+		void *start, *end;
+		char name[MAPS_LINE_LEN];
+
+		/* sscanf() is safe here as strlen(name) >= strlen(line) */
+		if (sscanf(line, "%p-%p %c-%cp %*x %*x:%*x %*u %s",
+			   &start, &end, &r, &x, name) != 5)
+			continue;
+
+		if (strcmp(name, "[vsyscall]"))
+			continue;
+
+		printf("\tvsyscall map: %s", line);
+
+		if (start != (void *)0xffffffffff600000 ||
+		    end != (void *)0xffffffffff601000) {
+			printf("[FAIL]\taddress range is nonsense\n");
+			nerrs++;
+		}
+
+		printf("\tvsyscall permissions are %c-%c\n", r, x);
+		vsyscall_map_r = (r == 'r');
+		vsyscall_map_x = (x == 'x');
+
+		found = true;
+		break;
+	}
+
+	fclose(maps);
+
+	if (!found) {
+		printf("\tno vsyscall map in /proc/self/maps\n");
+		vsyscall_map_r = false;
+		vsyscall_map_x = false;
+	}
+
+	return nerrs;
+}
+
 static volatile sig_atomic_t num_vsyscall_traps;
 
 static void sigtrap(int sig, siginfo_t *info, void *ctx_void)
@@ -559,20 +549,20 @@  int main(int argc, char **argv)
 	int nerrs = 0;
 
 	init_vdso();
+#ifdef __x86_64__
 	nerrs += init_vsys();
+#endif
 
 	nerrs += test_gtod();
 	nerrs += test_time();
 	nerrs += test_getcpu(0);
 	nerrs += test_getcpu(1);
 
+#ifdef __x86_64__
 	sethandler(SIGSEGV, sigsegv, 0);
 	nerrs += test_vsys_r();
 	nerrs += test_vsys_x();
-
 	nerrs += test_process_vm_readv();
-
-#ifdef __x86_64__
 	nerrs += test_emulation();
 #endif