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

Daniel Vetter
2019-02-20T23:39:01Z

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