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

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

 



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