[kvm-unit-tests PATCH v2 0/4] arm/powerpc: make argv[0] the program name

[kvm-unit-tests PATCH v2 0/4] arm/powerpc: make argv[0] the program name

[Andrew Jones]

v2:
 - copy auxinfo.prognam into argv[0] [drew]


It just came to my attention that x86 has the normal argv[0], the
program name, thanks to seabios. That raised the priority of fixing
one of my pet peeves - arm and powerpc starting arguments
at argv[0], as that just ain't right. This series fixes that. A few
temporary hacks are used to avoid one big patch fixing both arm
and powerpc at the same time. The hacks are gone by the end of the
series.


Andrew Jones (4):
  arm/arm64: reserve argv[0] for prognam
  powerpc/ppc64: reserve argv[0] for prognam
  arm/arm64: populate argv[0] with prognam
  powerpc/ppc64: populate argv[0] with prognam

 arm/Makefile.common     |  6 +++++-
 arm/selftest.c          | 14 +++++++-------
 arm/spinlock-test.c     |  2 +-
 lib/argv.c              | 31 +++++++++++++++++++++++--------
 lib/auxinfo.h           |  7 +++++++
 powerpc/Makefile.common |  6 +++++-
 powerpc/emulator.c      |  2 +-
 powerpc/rtas.c          | 12 ++++++------
 powerpc/selftest.c      |  8 ++++----
 powerpc/spapr_hcall.c   |  6 +++---
 scripts/auxinfo.mak     |  7 +++++++
 11 files changed, 69 insertions(+), 32 deletions(-)
 create mode 100644 lib/auxinfo.h
 create mode 100755 scripts/auxinfo.mak

-- 
2.4.11

[kvm-unit-tests PATCH v2 1/4] arm/arm64: reserve argv[0] for prognam

[Andrew Jones]

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arm/selftest.c      | 14 +++++++-------
 arm/spinlock-test.c |  2 +-
 lib/argv.c          | 16 ++++++++++++----
 3 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/arm/selftest.c b/arm/selftest.c
index 75dc91faab69a..5656f2bb1cc88 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -324,25 +324,25 @@ int main(int argc, char **argv)
 {
 	report_prefix_push("selftest");
 
-	if (!argc)
+	if (argc < 2)
 		report_abort("no test specified");
 
-	report_prefix_push(argv[0]);
+	report_prefix_push(argv[1]);
 
-	if (strcmp(argv[0], "setup") == 0) {
+	if (strcmp(argv[1], "setup") == 0) {
 
-		check_setup(argc-1, &argv[1]);
+		check_setup(argc-2, &argv[2]);
 
-	} else if (strcmp(argv[0], "vectors-kernel") == 0) {
+	} else if (strcmp(argv[1], "vectors-kernel") == 0) {
 
 		check_vectors(NULL);
 
-	} else if (strcmp(argv[0], "vectors-user") == 0) {
+	} else if (strcmp(argv[1], "vectors-user") == 0) {
 
 		start_usr(check_vectors, NULL,
 				(unsigned long)thread_stack_alloc());
 
-	} else if (strcmp(argv[0], "smp") == 0) {
+	} else if (strcmp(argv[1], "smp") == 0) {
 
 		int cpu;
 
diff --git a/arm/spinlock-test.c b/arm/spinlock-test.c
index fd2af9fd2f4d3..6009ba087e4b4 100644
--- a/arm/spinlock-test.c
+++ b/arm/spinlock-test.c
@@ -69,7 +69,7 @@ int main(int argc, char **argv)
 {
 	int cpu;
 
-	if (argc && strcmp(argv[0], "bad") != 0) {
+	if (argc > 1 && strcmp(argv[1], "bad") != 0) {
 		lock_ops.lock = gcc_builtin_lock;
 		lock_ops.unlock = gcc_builtin_unlock;
 	} else {
diff --git a/lib/argv.c b/lib/argv.c
index 62dd1fd4cf980..1c6c6a44c836d 100644
--- a/lib/argv.c
+++ b/lib/argv.c
@@ -34,9 +34,17 @@ void __setup_args(void)
 
 void setup_args(char *args)
 {
-    if (!args)
-        return;
+    if (args) {
+        __args = args;
+        __setup_args();
 
-    __args = args;
-    __setup_args();
+#if defined(__arm__) || defined(__aarch64__)
+        for (int i = __argc; i > 0; --i)
+            __argv[i] = __argv[i-1];
+#endif
+    }
+#if defined(__arm__) || defined(__aarch64__)
+    __argv[0] = NULL; //HACK: just reserve argv[0] for now
+    ++__argc;
+#endif
 }
-- 
2.4.11

[kvm-unit-tests PATCH v2 2/4] powerpc/ppc64: reserve argv[0] for prognam

[Andrew Jones]

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 lib/argv.c            |  4 ----
 powerpc/emulator.c    |  2 +-
 powerpc/rtas.c        | 12 ++++++------
 powerpc/selftest.c    |  8 ++++----
 powerpc/spapr_hcall.c |  6 +++---
 5 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/lib/argv.c b/lib/argv.c
index 1c6c6a44c836d..c6ad5fcbc8cf4 100644
--- a/lib/argv.c
+++ b/lib/argv.c
@@ -38,13 +38,9 @@ void setup_args(char *args)
         __args = args;
         __setup_args();
 
-#if defined(__arm__) || defined(__aarch64__)
         for (int i = __argc; i > 0; --i)
             __argv[i] = __argv[i-1];
-#endif
     }
-#if defined(__arm__) || defined(__aarch64__)
     __argv[0] = NULL; //HACK: just reserve argv[0] for now
     ++__argc;
-#endif
 }
diff --git a/powerpc/emulator.c b/powerpc/emulator.c
index 3696d839b2f6f..25018a57463b7 100644
--- a/powerpc/emulator.c
+++ b/powerpc/emulator.c
@@ -224,7 +224,7 @@ int main(int argc, char **argv)
 	handle_exception(0x700, program_check_handler, (void *)&is_invalid);
 	handle_exception(0x600, alignment_handler, (void *)&alignment);
 
-	for (i = 0; i < argc; i++) {
+	for (i = 1; i < argc; i++) {
 		if (strcmp(argv[i], "-v") == 0) {
 			verbose = 1;
 		}
diff --git a/powerpc/rtas.c b/powerpc/rtas.c
index 9d673f0ce8d93..1b1e9c753ef1b 100644
--- a/powerpc/rtas.c
+++ b/powerpc/rtas.c
@@ -115,23 +115,23 @@ int main(int argc, char **argv)
 
 	report_prefix_push("rtas");
 
-	if (!argc)
+	if (argc < 2)
 		report_abort("no test specified");
 
-	report_prefix_push(argv[0]);
+	report_prefix_push(argv[1]);
 
-	if (strcmp(argv[0], "get-time-of-day") == 0) {
+	if (strcmp(argv[1], "get-time-of-day") == 0) {
 
-		len = parse_keyval(argv[1], &val);
+		len = parse_keyval(argv[2], &val);
 		if (len == -1) {
 			printf("Missing parameter \"date\"\n");
 			abort();
 		}
-		argv[1][len] = '\0';
+		argv[2][len] = '\0';
 
 		check_get_time_of_day(val);
 
-	} else if (strcmp(argv[0], "set-time-of-day") == 0) {
+	} else if (strcmp(argv[1], "set-time-of-day") == 0) {
 
 		check_set_time_of_day();
 
diff --git a/powerpc/selftest.c b/powerpc/selftest.c
index 84867e482d2a2..8c5ff0ac889d4 100644
--- a/powerpc/selftest.c
+++ b/powerpc/selftest.c
@@ -49,14 +49,14 @@ int main(int argc, char **argv)
 {
 	report_prefix_push("selftest");
 
-	if (!argc)
+	if (argc < 2)
 		report_abort("no test specified");
 
-	report_prefix_push(argv[0]);
+	report_prefix_push(argv[1]);
 
-	if (strcmp(argv[0], "setup") == 0) {
+	if (strcmp(argv[1], "setup") == 0) {
 
-		check_setup(argc-1, &argv[1]);
+		check_setup(argc-2, &argv[2]);
 
 	}
 
diff --git a/powerpc/spapr_hcall.c b/powerpc/spapr_hcall.c
index dbff63013297b..656aaff61405b 100644
--- a/powerpc/spapr_hcall.c
+++ b/powerpc/spapr_hcall.c
@@ -154,13 +154,13 @@ int main(int argc, char **argv)
 
 	report_prefix_push("hypercall");
 
-	if (!argc || (argc == 1 && !strcmp(argv[0], "all")))
+	if (argc < 2 || (argc == 2 && !strcmp(argv[1], "all")))
 		all = 1;
 
 	for (i = 0; hctests[i].name != NULL; i++) {
 		report_prefix_push(hctests[i].name);
-		if (all || strcmp(argv[0], hctests[i].name) == 0) {
-			hctests[i].func(argc, argv);
+		if (all || strcmp(argv[1], hctests[i].name) == 0) {
+			hctests[i].func(argc-1, &argv[1]);
 		}
 		report_prefix_pop();
 	}
-- 
2.4.11

[kvm-unit-tests PATCH v2 3/4] arm/arm64: populate argv[0] with prognam

[Andrew Jones]

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arm/Makefile.common |  6 +++++-
 lib/argv.c          | 25 +++++++++++++++++++++----
 lib/auxinfo.h       |  7 +++++++
 scripts/auxinfo.mak |  7 +++++++
 4 files changed, 40 insertions(+), 5 deletions(-)
 create mode 100644 lib/auxinfo.h
 create mode 100755 scripts/auxinfo.mak

diff --git a/arm/Makefile.common b/arm/Makefile.common
index 9a2d61fc88a27..6b87a48b0066b 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -26,6 +26,7 @@ CFLAGS += -I lib -I lib/libfdt
 
 asm-offsets = lib/$(ARCH)/asm-offsets.h
 include scripts/asm-offsets.mak
+include scripts/auxinfo.mak
 
 cflatobjs += lib/util.o
 cflatobjs += lib/alloc.o
@@ -49,9 +50,12 @@ start_addr := $(shell printf "%x\n" $$(( $(phys_base) + $(kernel_offset) )))
 FLATLIBS = $(libcflat) $(LIBFDT_archive) $(libgcc) $(libeabi)
 %.elf: LDFLAGS = $(CFLAGS) -nostdlib
 %.elf: %.o $(FLATLIBS) arm/flat.lds
+	$(call gen-auxinfo,$(@:.elf=.aux.c),$(@:.elf=.flat))
+	$(CC) $(CFLAGS) -c -o $(@:.elf=.aux.o) $(@:.elf=.aux.c)
 	$(CC) $(LDFLAGS) -o $@ \
 		-Wl,-T,arm/flat.lds,--build-id=none,-Ttext=$(start_addr) \
-		$(filter %.o, $^) $(FLATLIBS)
+		$(filter %.o, $^) $(FLATLIBS) $(@:.elf=.aux.o)
+	$(RM) $(@:.elf=.aux).*
 
 %.flat: %.elf
 	$(OBJCOPY) -O binary $^ $@
diff --git a/lib/argv.c b/lib/argv.c
index c6ad5fcbc8cf4..66cd43f24d336 100644
--- a/lib/argv.c
+++ b/lib/argv.c
@@ -1,4 +1,7 @@
 #include "libcflat.h"
+#include "auxinfo.h"
+
+static char *copy_ptr;
 
 int __argc;
 char *__argv[100];
@@ -21,26 +24,40 @@ void __setup_args(void)
 {
     char *args = __args;
     char **argv = __argv;
-    char *p = __args_copy;
+
+    copy_ptr = __args_copy;
 
     while (*(args = skip_blanks(args)) != '\0') {
-        *argv++ = p;
+        *argv++ = copy_ptr;
         while (*args != '\0' && !isblank(*args))
-            *p++ = *args++;
-        *p++ = '\0';
+            *copy_ptr++ = *args++;
+        *copy_ptr++ = '\0';
     }
     __argc = argv - __argv;
 }
 
 void setup_args(char *args)
 {
+#if defined(__arm__) || defined(__aarch64__)
+    const char *p;
+#endif
+
     if (args) {
         __args = args;
         __setup_args();
 
         for (int i = __argc; i > 0; --i)
             __argv[i] = __argv[i-1];
+    } else {
+        copy_ptr = __args_copy;
     }
+#if defined(__arm__) || defined(__aarch64__)
+    __argv[0] = copy_ptr;
+    p = auxinfo.prognam;
+    while ((*copy_ptr++ = *p++) != 0)
+        ;
+#else
     __argv[0] = NULL; //HACK: just reserve argv[0] for now
+#endif
     ++__argc;
 }
diff --git a/lib/auxinfo.h b/lib/auxinfo.h
new file mode 100644
index 0000000000000..fc2d736aa63b1
--- /dev/null
+++ b/lib/auxinfo.h
@@ -0,0 +1,7 @@
+#ifndef _AUXINFO_H_
+#define _AUXINFO_H_
+struct auxinfo {
+	const char *prognam;
+};
+extern struct auxinfo auxinfo;
+#endif
diff --git a/scripts/auxinfo.mak b/scripts/auxinfo.mak
new file mode 100755
index 0000000000000..dbb588e89fc6f
--- /dev/null
+++ b/scripts/auxinfo.mak
@@ -0,0 +1,7 @@
+
+define gen-auxinfo
+	(echo "#include <auxinfo.h>";		\
+	 echo "struct auxinfo auxinfo = {";	\
+	 echo "    .prognam = \"$(2)\",";	\
+	 echo "};" ) > $(1)
+endef
-- 
2.4.11

[kvm-unit-tests PATCH v2 4/4] powerpc/ppc64: populate argv[0] with prognam

[Andrew Jones]

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 lib/argv.c              | 6 ------
 powerpc/Makefile.common | 6 +++++-
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/lib/argv.c b/lib/argv.c
index 66cd43f24d336..63b9ab2af2d2a 100644
--- a/lib/argv.c
+++ b/lib/argv.c
@@ -38,9 +38,7 @@ void __setup_args(void)
 
 void setup_args(char *args)
 {
-#if defined(__arm__) || defined(__aarch64__)
     const char *p;
-#endif
 
     if (args) {
         __args = args;
@@ -51,13 +49,9 @@ void setup_args(char *args)
     } else {
         copy_ptr = __args_copy;
     }
-#if defined(__arm__) || defined(__aarch64__)
     __argv[0] = copy_ptr;
     p = auxinfo.prognam;
     while ((*copy_ptr++ = *p++) != 0)
         ;
-#else
-    __argv[0] = NULL; //HACK: just reserve argv[0] for now
-#endif
     ++__argc;
 }
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 4449aec616853..150f5fc06bc2b 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -24,6 +24,7 @@ CFLAGS += -fpie
 
 asm-offsets = lib/$(ARCH)/asm-offsets.h
 include scripts/asm-offsets.mak
+include scripts/auxinfo.mak
 
 cflatobjs += lib/util.o
 cflatobjs += lib/alloc.o
@@ -38,9 +39,12 @@ FLATLIBS = $(libcflat) $(LIBFDT_archive)
 %.elf: CFLAGS += $(arch_CFLAGS)
 %.elf: LDFLAGS = $(arch_LDFLAGS) -nostdlib -pie
 %.elf: %.o $(FLATLIBS) powerpc/flat.lds
+	$(call gen-auxinfo,$(@:.elf=.aux.c),$@)
+	$(CC) $(CFLAGS) -c -o $(@:.elf=.aux.o) $(@:.elf=.aux.c)
 	$(LD) $(LDFLAGS) -o $@ \
 	      -T powerpc/flat.lds --build-id=none \
-		$(filter %.o, $^) $(FLATLIBS)
+		$(filter %.o, $^) $(FLATLIBS) $(@:.elf=.aux.o)
+	$(RM) $(@:.elf=.aux).*
 	@echo -n Checking $@ for unsupported reloc types...
 	@if $(OBJDUMP) -R $@ | grep R_ | grep -v R_PPC64_RELATIVE; then	\
 		false;							\
-- 
2.4.11

Re: [kvm-unit-tests PATCH v2 1/4] arm/arm64: reserve argv[0] for prognam

[Thomas Huth]

On 22.04.2016 16:54, Andrew Jones wrote:
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  arm/selftest.c      | 14 +++++++-------
>  arm/spinlock-test.c |  2 +-
>  lib/argv.c          | 16 ++++++++++++----
>  3 files changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/arm/selftest.c b/arm/selftest.c
> index 75dc91faab69a..5656f2bb1cc88 100644
> --- a/arm/selftest.c
> +++ b/arm/selftest.c
> @@ -324,25 +324,25 @@ int main(int argc, char **argv)
>  {
>  	report_prefix_push("selftest");
>  
> -	if (!argc)
> +	if (argc < 2)
>  		report_abort("no test specified");
>  
> -	report_prefix_push(argv[0]);
> +	report_prefix_push(argv[1]);
>  
> -	if (strcmp(argv[0], "setup") == 0) {
> +	if (strcmp(argv[1], "setup") == 0) {
>  
> -		check_setup(argc-1, &argv[1]);
> +		check_setup(argc-2, &argv[2]);
>  
> -	} else if (strcmp(argv[0], "vectors-kernel") == 0) {
> +	} else if (strcmp(argv[1], "vectors-kernel") == 0) {
>  
>  		check_vectors(NULL);
>  
> -	} else if (strcmp(argv[0], "vectors-user") == 0) {
> +	} else if (strcmp(argv[1], "vectors-user") == 0) {
>  
>  		start_usr(check_vectors, NULL,
>  				(unsigned long)thread_stack_alloc());
>  
> -	} else if (strcmp(argv[0], "smp") == 0) {
> +	} else if (strcmp(argv[1], "smp") == 0) {
>  
>  		int cpu;
>  
> diff --git a/arm/spinlock-test.c b/arm/spinlock-test.c
> index fd2af9fd2f4d3..6009ba087e4b4 100644
> --- a/arm/spinlock-test.c
> +++ b/arm/spinlock-test.c
> @@ -69,7 +69,7 @@ int main(int argc, char **argv)
>  {
>  	int cpu;
>  
> -	if (argc && strcmp(argv[0], "bad") != 0) {
> +	if (argc > 1 && strcmp(argv[1], "bad") != 0) {
>  		lock_ops.lock = gcc_builtin_lock;
>  		lock_ops.unlock = gcc_builtin_unlock;
>  	} else {
> diff --git a/lib/argv.c b/lib/argv.c
> index 62dd1fd4cf980..1c6c6a44c836d 100644
> --- a/lib/argv.c
> +++ b/lib/argv.c
> @@ -34,9 +34,17 @@ void __setup_args(void)
>  
>  void setup_args(char *args)
>  {
> -    if (!args)
> -        return;
> +    if (args) {
> +        __args = args;
> +        __setup_args();
>  
> -    __args = args;
> -    __setup_args();
> +#if defined(__arm__) || defined(__aarch64__)
> +        for (int i = __argc; i > 0; --i)

Declaring a variable within a for statement is C++ style ... but since
it is used in a couple of places of kvm-unit-test already, I assume this
is ok?

> +            __argv[i] = __argv[i-1];
> +#endif
> +    }
> +#if defined(__arm__) || defined(__aarch64__)
> +    __argv[0] = NULL; //HACK: just reserve argv[0] for now
> +    ++__argc;
> +#endif
>  }

Looks fine to me (apart from that C++ nit ;-))

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [kvm-unit-tests PATCH v2 2/4] powerpc/ppc64: reserve argv[0] for prognam

[Thomas Huth]

On 22.04.2016 16:54, Andrew Jones wrote:
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  lib/argv.c            |  4 ----
>  powerpc/emulator.c    |  2 +-
>  powerpc/rtas.c        | 12 ++++++------
>  powerpc/selftest.c    |  8 ++++----
>  powerpc/spapr_hcall.c |  6 +++---
>  5 files changed, 14 insertions(+), 18 deletions(-)
> 
> diff --git a/lib/argv.c b/lib/argv.c
> index 1c6c6a44c836d..c6ad5fcbc8cf4 100644
> --- a/lib/argv.c
> +++ b/lib/argv.c
> @@ -38,13 +38,9 @@ void setup_args(char *args)
>          __args = args;
>          __setup_args();
>  
> -#if defined(__arm__) || defined(__aarch64__)
>          for (int i = __argc; i > 0; --i)
>              __argv[i] = __argv[i-1];
> -#endif
>      }
> -#if defined(__arm__) || defined(__aarch64__)
>      __argv[0] = NULL; //HACK: just reserve argv[0] for now
>      ++__argc;
> -#endif
>  }
> diff --git a/powerpc/emulator.c b/powerpc/emulator.c
> index 3696d839b2f6f..25018a57463b7 100644
> --- a/powerpc/emulator.c
> +++ b/powerpc/emulator.c
> @@ -224,7 +224,7 @@ int main(int argc, char **argv)
>  	handle_exception(0x700, program_check_handler, (void *)&is_invalid);
>  	handle_exception(0x600, alignment_handler, (void *)&alignment);
>  
> -	for (i = 0; i < argc; i++) {
> +	for (i = 1; i < argc; i++) {
>  		if (strcmp(argv[i], "-v") == 0) {
>  			verbose = 1;
>  		}
> diff --git a/powerpc/rtas.c b/powerpc/rtas.c
> index 9d673f0ce8d93..1b1e9c753ef1b 100644
> --- a/powerpc/rtas.c
> +++ b/powerpc/rtas.c
> @@ -115,23 +115,23 @@ int main(int argc, char **argv)
>  
>  	report_prefix_push("rtas");
>  
> -	if (!argc)
> +	if (argc < 2)
>  		report_abort("no test specified");
>  
> -	report_prefix_push(argv[0]);
> +	report_prefix_push(argv[1]);
>  
> -	if (strcmp(argv[0], "get-time-of-day") == 0) {
> +	if (strcmp(argv[1], "get-time-of-day") == 0) {
>  
> -		len = parse_keyval(argv[1], &val);
> +		len = parse_keyval(argv[2], &val);
>  		if (len == -1) {
>  			printf("Missing parameter \"date\"\n");
>  			abort();
>  		}
> -		argv[1][len] = '\0';
> +		argv[2][len] = '\0';
>  
>  		check_get_time_of_day(val);
>  
> -	} else if (strcmp(argv[0], "set-time-of-day") == 0) {
> +	} else if (strcmp(argv[1], "set-time-of-day") == 0) {
>  
>  		check_set_time_of_day();
>  
> diff --git a/powerpc/selftest.c b/powerpc/selftest.c
> index 84867e482d2a2..8c5ff0ac889d4 100644
> --- a/powerpc/selftest.c
> +++ b/powerpc/selftest.c
> @@ -49,14 +49,14 @@ int main(int argc, char **argv)
>  {
>  	report_prefix_push("selftest");
>  
> -	if (!argc)
> +	if (argc < 2)
>  		report_abort("no test specified");
>  
> -	report_prefix_push(argv[0]);
> +	report_prefix_push(argv[1]);
>  
> -	if (strcmp(argv[0], "setup") == 0) {
> +	if (strcmp(argv[1], "setup") == 0) {
>  
> -		check_setup(argc-1, &argv[1]);
> +		check_setup(argc-2, &argv[2]);
>  
>  	}
>  
> diff --git a/powerpc/spapr_hcall.c b/powerpc/spapr_hcall.c
> index dbff63013297b..656aaff61405b 100644
> --- a/powerpc/spapr_hcall.c
> +++ b/powerpc/spapr_hcall.c
> @@ -154,13 +154,13 @@ int main(int argc, char **argv)
>  
>  	report_prefix_push("hypercall");
>  
> -	if (!argc || (argc == 1 && !strcmp(argv[0], "all")))
> +	if (argc < 2 || (argc == 2 && !strcmp(argv[1], "all")))
>  		all = 1;
>  
>  	for (i = 0; hctests[i].name != NULL; i++) {
>  		report_prefix_push(hctests[i].name);
> -		if (all || strcmp(argv[0], hctests[i].name) == 0) {
> -			hctests[i].func(argc, argv);
> +		if (all || strcmp(argv[1], hctests[i].name) == 0) {
> +			hctests[i].func(argc-1, &argv[1]);
>  		}
>  		report_prefix_pop();
>  	}

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [kvm-unit-tests PATCH v2 1/4] arm/arm64: reserve argv[0] for prognam

[Andrew Jones]

On Fri, Apr 22, 2016 at 07:23:16PM +0200, Thomas Huth wrote:
> On 22.04.2016 16:54, Andrew Jones wrote:
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  arm/selftest.c      | 14 +++++++-------
> >  arm/spinlock-test.c |  2 +-
> >  lib/argv.c          | 16 ++++++++++++----
> >  3 files changed, 20 insertions(+), 12 deletions(-)
> > 
> > diff --git a/arm/selftest.c b/arm/selftest.c
> > index 75dc91faab69a..5656f2bb1cc88 100644
> > --- a/arm/selftest.c
> > +++ b/arm/selftest.c
> > @@ -324,25 +324,25 @@ int main(int argc, char **argv)
> >  {
> >  	report_prefix_push("selftest");
> >  
> > -	if (!argc)
> > +	if (argc < 2)
> >  		report_abort("no test specified");
> >  
> > -	report_prefix_push(argv[0]);
> > +	report_prefix_push(argv[1]);
> >  
> > -	if (strcmp(argv[0], "setup") == 0) {
> > +	if (strcmp(argv[1], "setup") == 0) {
> >  
> > -		check_setup(argc-1, &argv[1]);
> > +		check_setup(argc-2, &argv[2]);
> >  
> > -	} else if (strcmp(argv[0], "vectors-kernel") == 0) {
> > +	} else if (strcmp(argv[1], "vectors-kernel") == 0) {
> >  
> >  		check_vectors(NULL);
> >  
> > -	} else if (strcmp(argv[0], "vectors-user") == 0) {
> > +	} else if (strcmp(argv[1], "vectors-user") == 0) {
> >  
> >  		start_usr(check_vectors, NULL,
> >  				(unsigned long)thread_stack_alloc());
> >  
> > -	} else if (strcmp(argv[0], "smp") == 0) {
> > +	} else if (strcmp(argv[1], "smp") == 0) {
> >  
> >  		int cpu;
> >  
> > diff --git a/arm/spinlock-test.c b/arm/spinlock-test.c
> > index fd2af9fd2f4d3..6009ba087e4b4 100644
> > --- a/arm/spinlock-test.c
> > +++ b/arm/spinlock-test.c
> > @@ -69,7 +69,7 @@ int main(int argc, char **argv)
> >  {
> >  	int cpu;
> >  
> > -	if (argc && strcmp(argv[0], "bad") != 0) {
> > +	if (argc > 1 && strcmp(argv[1], "bad") != 0) {
> >  		lock_ops.lock = gcc_builtin_lock;
> >  		lock_ops.unlock = gcc_builtin_unlock;
> >  	} else {
> > diff --git a/lib/argv.c b/lib/argv.c
> > index 62dd1fd4cf980..1c6c6a44c836d 100644
> > --- a/lib/argv.c
> > +++ b/lib/argv.c
> > @@ -34,9 +34,17 @@ void __setup_args(void)
> >  
> >  void setup_args(char *args)
> >  {
> > -    if (!args)
> > -        return;
> > +    if (args) {
> > +        __args = args;
> > +        __setup_args();
> >  
> > -    __args = args;
> > -    __setup_args();
> > +#if defined(__arm__) || defined(__aarch64__)
> > +        for (int i = __argc; i > 0; --i)
> 
> Declaring a variable within a for statement is C++ style ... but since
> it is used in a couple of places of kvm-unit-test already, I assume this
> is ok?

I usually don't use them for the reason you've stated, but this time
I didn't want to add the #ifdef at the top of the function just for
the 'int i'. Of course I had to add an #ifdef in a later patch for
a different variable though, so I guess I shouldn't have been so lazy.
I'll change it if I go to v3.

> 
> > +            __argv[i] = __argv[i-1];
> > +#endif
> > +    }
> > +#if defined(__arm__) || defined(__aarch64__)
> > +    __argv[0] = NULL; //HACK: just reserve argv[0] for now
> > +    ++__argc;
> > +#endif
> >  }
> 
> Looks fine to me (apart from that C++ nit ;-))
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Thanks,
drew

> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kvm-unit-tests PATCH v2 3/4] arm/arm64: populate argv[0] with prognam

[Thomas Huth]

On 22.04.2016 16:54, Andrew Jones wrote:
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  arm/Makefile.common |  6 +++++-
>  lib/argv.c          | 25 +++++++++++++++++++++----
>  lib/auxinfo.h       |  7 +++++++
>  scripts/auxinfo.mak |  7 +++++++
>  4 files changed, 40 insertions(+), 5 deletions(-)
>  create mode 100644 lib/auxinfo.h
>  create mode 100755 scripts/auxinfo.mak
> 
> diff --git a/arm/Makefile.common b/arm/Makefile.common
> index 9a2d61fc88a27..6b87a48b0066b 100644
> --- a/arm/Makefile.common
> +++ b/arm/Makefile.common
> @@ -26,6 +26,7 @@ CFLAGS += -I lib -I lib/libfdt
>  
>  asm-offsets = lib/$(ARCH)/asm-offsets.h
>  include scripts/asm-offsets.mak
> +include scripts/auxinfo.mak
>  
>  cflatobjs += lib/util.o
>  cflatobjs += lib/alloc.o
> @@ -49,9 +50,12 @@ start_addr := $(shell printf "%x\n" $$(( $(phys_base) + $(kernel_offset) )))
>  FLATLIBS = $(libcflat) $(LIBFDT_archive) $(libgcc) $(libeabi)
>  %.elf: LDFLAGS = $(CFLAGS) -nostdlib
>  %.elf: %.o $(FLATLIBS) arm/flat.lds
> +	$(call gen-auxinfo,$(@:.elf=.aux.c),$(@:.elf=.flat))
> +	$(CC) $(CFLAGS) -c -o $(@:.elf=.aux.o) $(@:.elf=.aux.c)
>  	$(CC) $(LDFLAGS) -o $@ \
>  		-Wl,-T,arm/flat.lds,--build-id=none,-Ttext=$(start_addr) \
> -		$(filter %.o, $^) $(FLATLIBS)
> +		$(filter %.o, $^) $(FLATLIBS) $(@:.elf=.aux.o)
> +	$(RM) $(@:.elf=.aux).*
>  
>  %.flat: %.elf
>  	$(OBJCOPY) -O binary $^ $@
> diff --git a/lib/argv.c b/lib/argv.c
> index c6ad5fcbc8cf4..66cd43f24d336 100644
> --- a/lib/argv.c
> +++ b/lib/argv.c
> @@ -1,4 +1,7 @@
>  #include "libcflat.h"
> +#include "auxinfo.h"
> +
> +static char *copy_ptr;
>  
>  int __argc;
>  char *__argv[100];
> @@ -21,26 +24,40 @@ void __setup_args(void)
>  {
>      char *args = __args;
>      char **argv = __argv;
> -    char *p = __args_copy;
> +
> +    copy_ptr = __args_copy;
>  
>      while (*(args = skip_blanks(args)) != '\0') {
> -        *argv++ = p;
> +        *argv++ = copy_ptr;
>          while (*args != '\0' && !isblank(*args))
> -            *p++ = *args++;
> -        *p++ = '\0';
> +            *copy_ptr++ = *args++;
> +        *copy_ptr++ = '\0';
>      }
>      __argc = argv - __argv;
>  }
>  
>  void setup_args(char *args)
>  {
> +#if defined(__arm__) || defined(__aarch64__)
> +    const char *p;
> +#endif
> +
>      if (args) {
>          __args = args;
>          __setup_args();
>  
>          for (int i = __argc; i > 0; --i)
>              __argv[i] = __argv[i-1];
> +    } else {
> +        copy_ptr = __args_copy;
>      }
> +#if defined(__arm__) || defined(__aarch64__)
> +    __argv[0] = copy_ptr;
> +    p = auxinfo.prognam;
> +    while ((*copy_ptr++ = *p++) != 0)
> +        ;

Could you also use strcpy() here? Or do you still need the updated
copy_ptr afterwards?

> +#else
>      __argv[0] = NULL; //HACK: just reserve argv[0] for now
> +#endif
>      ++__argc;
>  }
> diff --git a/lib/auxinfo.h b/lib/auxinfo.h
> new file mode 100644
> index 0000000000000..fc2d736aa63b1
> --- /dev/null
> +++ b/lib/auxinfo.h
> @@ -0,0 +1,7 @@
> +#ifndef _AUXINFO_H_
> +#define _AUXINFO_H_
> +struct auxinfo {
> +	const char *prognam;
> +};
> +extern struct auxinfo auxinfo;
> +#endif
> diff --git a/scripts/auxinfo.mak b/scripts/auxinfo.mak
> new file mode 100755
> index 0000000000000..dbb588e89fc6f
> --- /dev/null
> +++ b/scripts/auxinfo.mak
> @@ -0,0 +1,7 @@
> +
> +define gen-auxinfo
> +	(echo "#include <auxinfo.h>";		\
> +	 echo "struct auxinfo auxinfo = {";	\
> +	 echo "    .prognam = \"$(2)\",";	\
> +	 echo "};" ) > $(1)
> +endef

Looks sane. Not sure, whether it really makes sense to provide the name
to the unit test, but hey, why not?

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [kvm-unit-tests PATCH v2 4/4] powerpc/ppc64: populate argv[0] with prognam

[Thomas Huth]

On 22.04.2016 16:54, Andrew Jones wrote:
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  lib/argv.c              | 6 ------
>  powerpc/Makefile.common | 6 +++++-
>  2 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/argv.c b/lib/argv.c
> index 66cd43f24d336..63b9ab2af2d2a 100644
> --- a/lib/argv.c
> +++ b/lib/argv.c
> @@ -38,9 +38,7 @@ void __setup_args(void)
>  
>  void setup_args(char *args)
>  {
> -#if defined(__arm__) || defined(__aarch64__)
>      const char *p;
> -#endif
>  
>      if (args) {
>          __args = args;
> @@ -51,13 +49,9 @@ void setup_args(char *args)
>      } else {
>          copy_ptr = __args_copy;
>      }
> -#if defined(__arm__) || defined(__aarch64__)
>      __argv[0] = copy_ptr;
>      p = auxinfo.prognam;
>      while ((*copy_ptr++ = *p++) != 0)
>          ;
> -#else
> -    __argv[0] = NULL; //HACK: just reserve argv[0] for now
> -#endif
>      ++__argc;
>  }
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index 4449aec616853..150f5fc06bc2b 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -24,6 +24,7 @@ CFLAGS += -fpie
>  
>  asm-offsets = lib/$(ARCH)/asm-offsets.h
>  include scripts/asm-offsets.mak
> +include scripts/auxinfo.mak
>  
>  cflatobjs += lib/util.o
>  cflatobjs += lib/alloc.o
> @@ -38,9 +39,12 @@ FLATLIBS = $(libcflat) $(LIBFDT_archive)
>  %.elf: CFLAGS += $(arch_CFLAGS)
>  %.elf: LDFLAGS = $(arch_LDFLAGS) -nostdlib -pie
>  %.elf: %.o $(FLATLIBS) powerpc/flat.lds
> +	$(call gen-auxinfo,$(@:.elf=.aux.c),$@)
> +	$(CC) $(CFLAGS) -c -o $(@:.elf=.aux.o) $(@:.elf=.aux.c)
>  	$(LD) $(LDFLAGS) -o $@ \
>  	      -T powerpc/flat.lds --build-id=none \
> -		$(filter %.o, $^) $(FLATLIBS)
> +		$(filter %.o, $^) $(FLATLIBS) $(@:.elf=.aux.o)
> +	$(RM) $(@:.elf=.aux).*
>  	@echo -n Checking $@ for unsupported reloc types...
>  	@if $(OBJDUMP) -R $@ | grep R_ | grep -v R_PPC64_RELATIVE; then	\
>  		false;							\
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [kvm-unit-tests PATCH v2 3/4] arm/arm64: populate argv[0] with prognam

[Andrew Jones]

On Fri, Apr 22, 2016 at 07:48:28PM +0200, Thomas Huth wrote:
> On 22.04.2016 16:54, Andrew Jones wrote:
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  arm/Makefile.common |  6 +++++-
> >  lib/argv.c          | 25 +++++++++++++++++++++----
> >  lib/auxinfo.h       |  7 +++++++
> >  scripts/auxinfo.mak |  7 +++++++
> >  4 files changed, 40 insertions(+), 5 deletions(-)
> >  create mode 100644 lib/auxinfo.h
> >  create mode 100755 scripts/auxinfo.mak
> > 
> > diff --git a/arm/Makefile.common b/arm/Makefile.common
> > index 9a2d61fc88a27..6b87a48b0066b 100644
> > --- a/arm/Makefile.common
> > +++ b/arm/Makefile.common
> > @@ -26,6 +26,7 @@ CFLAGS += -I lib -I lib/libfdt
> >  
> >  asm-offsets = lib/$(ARCH)/asm-offsets.h
> >  include scripts/asm-offsets.mak
> > +include scripts/auxinfo.mak
> >  
> >  cflatobjs += lib/util.o
> >  cflatobjs += lib/alloc.o
> > @@ -49,9 +50,12 @@ start_addr := $(shell printf "%x\n" $$(( $(phys_base) + $(kernel_offset) )))
> >  FLATLIBS = $(libcflat) $(LIBFDT_archive) $(libgcc) $(libeabi)
> >  %.elf: LDFLAGS = $(CFLAGS) -nostdlib
> >  %.elf: %.o $(FLATLIBS) arm/flat.lds
> > +	$(call gen-auxinfo,$(@:.elf=.aux.c),$(@:.elf=.flat))
> > +	$(CC) $(CFLAGS) -c -o $(@:.elf=.aux.o) $(@:.elf=.aux.c)
> >  	$(CC) $(LDFLAGS) -o $@ \
> >  		-Wl,-T,arm/flat.lds,--build-id=none,-Ttext=$(start_addr) \
> > -		$(filter %.o, $^) $(FLATLIBS)
> > +		$(filter %.o, $^) $(FLATLIBS) $(@:.elf=.aux.o)
> > +	$(RM) $(@:.elf=.aux).*
> >  
> >  %.flat: %.elf
> >  	$(OBJCOPY) -O binary $^ $@
> > diff --git a/lib/argv.c b/lib/argv.c
> > index c6ad5fcbc8cf4..66cd43f24d336 100644
> > --- a/lib/argv.c
> > +++ b/lib/argv.c
> > @@ -1,4 +1,7 @@
> >  #include "libcflat.h"
> > +#include "auxinfo.h"
> > +
> > +static char *copy_ptr;
> >  
> >  int __argc;
> >  char *__argv[100];
> > @@ -21,26 +24,40 @@ void __setup_args(void)
> >  {
> >      char *args = __args;
> >      char **argv = __argv;
> > -    char *p = __args_copy;
> > +
> > +    copy_ptr = __args_copy;
> >  
> >      while (*(args = skip_blanks(args)) != '\0') {
> > -        *argv++ = p;
> > +        *argv++ = copy_ptr;
> >          while (*args != '\0' && !isblank(*args))
> > -            *p++ = *args++;
> > -        *p++ = '\0';
> > +            *copy_ptr++ = *args++;
> > +        *copy_ptr++ = '\0';
> >      }
> >      __argc = argv - __argv;
> >  }
> >  
> >  void setup_args(char *args)
> >  {
> > +#if defined(__arm__) || defined(__aarch64__)
> > +    const char *p;
> > +#endif
> > +
> >      if (args) {
> >          __args = args;
> >          __setup_args();
> >  
> >          for (int i = __argc; i > 0; --i)
> >              __argv[i] = __argv[i-1];
> > +    } else {
> > +        copy_ptr = __args_copy;
> >      }
> > +#if defined(__arm__) || defined(__aarch64__)
> > +    __argv[0] = copy_ptr;
> > +    p = auxinfo.prognam;
> > +    while ((*copy_ptr++ = *p++) != 0)
> > +        ;
> 
> Could you also use strcpy() here? Or do you still need the updated
> copy_ptr afterwards?

I considered strcpy, and not worrying about updating copy_ptr, but
then decided to make sure copy_ptr was left correct. You never know
when we might be hacking on this again (although atm I can't think
of why...)

> 
> > +#else
> >      __argv[0] = NULL; //HACK: just reserve argv[0] for now
> > +#endif
> >      ++__argc;
> >  }
> > diff --git a/lib/auxinfo.h b/lib/auxinfo.h
> > new file mode 100644
> > index 0000000000000..fc2d736aa63b1
> > --- /dev/null
> > +++ b/lib/auxinfo.h
> > @@ -0,0 +1,7 @@
> > +#ifndef _AUXINFO_H_
> > +#define _AUXINFO_H_
> > +struct auxinfo {
> > +	const char *prognam;
> > +};
> > +extern struct auxinfo auxinfo;
> > +#endif
> > diff --git a/scripts/auxinfo.mak b/scripts/auxinfo.mak
> > new file mode 100755
> > index 0000000000000..dbb588e89fc6f
> > --- /dev/null
> > +++ b/scripts/auxinfo.mak
> > @@ -0,0 +1,7 @@
> > +
> > +define gen-auxinfo
> > +	(echo "#include <auxinfo.h>";		\
> > +	 echo "struct auxinfo auxinfo = {";	\
> > +	 echo "    .prognam = \"$(2)\",";	\
> > +	 echo "};" ) > $(1)
> > +endef
> 
> Looks sane. Not sure, whether it really makes sense to provide the name
> to the unit test, but hey, why not?

Mostly to make sure argv[0] is reserved, so we can start input
parameters at argv[1] like a normal program. I considered just leaving
it NULL actually, but then decided we might as well populate it. Also,
now that we have this auxinfo thingy, we may find other uses for it.

> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Thanks,
drew

> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html