On Wed, Jul 09, 2014 at 01:50:30PM +0000, Gore, Tim wrote: > Some inline comments > > Tim > > > -----Original Message----- > > From: Daniel Vetter [mailto:daniel.vetter@xxxxxxxx] On Behalf Of Daniel > > Vetter > > Sent: Monday, July 07, 2014 5:12 PM > > To: Gore, Tim > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > Subject: Re: [PATCH 1/2] intel-gpu-tools: pass argc/argv to simple > > main > > > > On Fri, Jun 27, 2014 at 03:15:36PM +0100, tim.gore@xxxxxxxxx wrote: > > > From: Tim Gore <tim.gore@xxxxxxxxx> > > > > > > Quite a few single tests do not use the igt_simple_main macro because > > > they want access to argc/argv. So change the igt_simple_main macro to > > > pass these arguments through to the "__real_mainxxx" function, and > > > change these tests to use the macro. > > > > > > Signed-off-by: Tim Gore <tim.gore@xxxxxxxxx> > > > --- > > > lib/igt_core.h | 8 ++++---- > > > > > tests/gem_ctx_basic.c | 6 +----- > > > tests/gem_exec_blt.c | 5 +---- > > > tests/gem_gtt_speed.c | 5 +---- > > > tests/gem_hang.c | 5 +---- > > > tests/gem_render_copy.c | 4 +--- > > > tests/gem_render_linear_blits.c | 5 +---- > > > tests/gem_render_tiled_blits.c | 5 +---- > > > tests/gem_seqno_wrap.c | 11 ++++------- > > > tests/gem_stress.c | 5 +---- > > > tests/gen3_mixed_blits.c | 5 +---- > > > tests/gen3_render_linear_blits.c | 5 +---- > > > tests/gen3_render_mixed_blits.c | 5 +---- > > > tests/gen3_render_tiledx_blits.c | 5 +---- > > > tests/gen3_render_tiledy_blits.c | 5 +---- > > > 15 files changed, 21 insertions(+), 63 deletions(-) > > > > > > diff --git a/lib/igt_core.h b/lib/igt_core.h index e252eba..9853e6b > > > 100644 > > > --- a/lib/igt_core.h > > > +++ b/lib/igt_core.h > > > @@ -176,13 +176,13 @@ void igt_simple_init(void); > > > * the test needs to parse additional cmdline arguments of its own. > > > */ > > > #define igt_simple_main \ > > > - static void igt_tokencat(__real_main, __LINE__)(void); \ > > > + static void igt_tokencat(__real_main, __LINE__)(int argc, char > > > +**argv); \ > > > int main(int argc, char **argv) { \ > > > igt_simple_init(); \ > > > - igt_tokencat(__real_main, __LINE__)(); \ > > > - exit(0); \ > > > + igt_tokencat(__real_main, __LINE__)(argc, argv); \ > > > + igt_exit(); \ > > > } \ > > > - static void igt_tokencat(__real_main, __LINE__)(void) \ > > > + static void igt_tokencat(__real_main, __LINE__)(int argc, char > > > +**argv) \ > > > > Hm, I'd just add argc/argv to igt_simple_init like you do in the next patch and > > update the relevant tests which have their own main. That would be more in > > line with subtest-tests which have their own additional arguments, too. > > > > Also having functions with magic/predefined arguments is a bit too much > > magic for my taste. If we keep the signature of real_main as-is we'll avoid > > (highly unlikely, but still) accidental name clashes. > > > > And with my suggestion for patch 2 to just have a bare-bones argv parsing for > > simple tests we also don't need to call igt_exit. > > -Daniel > > My motivation here is mainly to reduce the amount of differentiation, so that > we can move away from having a different methodology for tests that want to > parse argv. Together with the next patch we move towards all tests having the > same setup/initialisation. I believe that this will make the test suite easier to > maintain and add features to, such as common Doxygen/--help text. > I agree that the passing argc/v through to real_main is not ideal, but the whole > macro thing is not to my taste to be honest, for all the usual reasons. If we can > reduce the number of test "types" (single/multiple , parses argv/or not) it > should make the macros easier to maintain etc. The problem is that simple tests without subtest do work slightly differently. One option we could pursue instead is to redefine igt_simple_main as igt_main igt_subtest("basic") but that will look a bit ugly in the test listenings. My proposal for this patch here was to add (argc, argv) parameters to igt_simple_init and then adjust just that function in patch 2. I don't see the upside of forcefully converting the tests to use the igt_simple_main macro - we also have subtest-tests with their open-coded main() function. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
