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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> > ---
> >  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@xxxxxxxxxx>

Thanks,
drew

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



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux