Re: [igt-dev] [PATCH i-g-t 1/3] lib/tests: Use ARRAY_SIZE

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Feb 20, 2019 at 05:00:11PM +0200, Arkadiusz Hiler wrote:
> On Wed, Feb 20, 2019 at 03:24:47PM +0100, Daniel Vetter wrote:
> > Except in igt_simulation.c where we use tricks and intentionally only
> > want part of the array in some cases.
> > 
> > Suggested-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  lib/tests/igt_assert.c               | 3 ++-
> >  lib/tests/igt_exit_handler.c         | 3 ++-
> >  lib/tests/igt_fork.c                 | 3 ++-
> >  lib/tests/igt_invalid_subtest_name.c | 5 +++--
> >  lib/tests/igt_list_only.c            | 2 +-
> >  lib/tests/igt_no_exit.c              | 4 ++--
> >  lib/tests/igt_no_subtest.c           | 2 +-
> >  lib/tests/igt_segfault.c             | 4 +---
> >  8 files changed, 14 insertions(+), 12 deletions(-)
> > 
> > diff --git a/lib/tests/igt_assert.c b/lib/tests/igt_assert.c
> > index 632e15978978..1caf5d885ddb 100644
> > --- a/lib/tests/igt_assert.c
> > +++ b/lib/tests/igt_assert.c
> > @@ -34,6 +34,7 @@
> >  #include <sys/wait.h>
> >  
> >  #include "igt_core.h"
> > +#include "drmtest.h"
> 
> It may be worth moving that macro somewhere else.
> ctags show me about 10 spearate definitions across the codebase.
> 
> But that's another cleanup.

Yeah I looked at git grep ARRAY_SIZE too, and the macro feels a bit
misplayed. But then got lazy :-)

> 
> > diff --git a/lib/tests/igt_segfault.c b/lib/tests/igt_segfault.c
> > index 2a24531aaa8c..0d872f67850a 100644
> > --- a/lib/tests/igt_segfault.c
> > +++ b/lib/tests/igt_segfault.c
> > @@ -65,15 +65,13 @@ static int do_fork(void)
> >  	case -1:
> >  		internal_assert(0);
> >  	case 0:
> > +		argc = ARRAY_SIZE(argv_run);
> >  		if (simple) {
> > -			argc = 1;
> >  			igt_simple_init(argc, argv_run);
> >  			crashme();
> >  
> >  			igt_exit();
> >  		} else {
> > -
> > -			argc = 1;
> >  			igt_subtest_init(argc, argv_run);
> >  
> 
> igt_simple_init(ARRAY_SIZE(argv_run), argv_run);
> may indeed be a bit much.

Because macro magic doesn't actually work, or something like that.
igt_subtest_init has a &argc which goes boom :-)

> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>

Thanks for your review, applied.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev