From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [igt PATCH 3/4] lib: add subtest extra command line option handling Date: Fri, 16 Aug 2013 14:07:07 +0300 Message-ID: <1376651227.2576.14.camel@intelbox> References: <1375703126-19323-1-git-send-email-imre.deak@intel.com> <1375703126-19323-4-git-send-email-imre.deak@intel.com> <20130806090944.GT22035@phenom.ffwll.local> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1458323504==" Return-path: Received: from mga14.intel.com (mga14.intel.com [143.182.124.37]) by gabe.freedesktop.org (Postfix) with ESMTP id A7B0BE5DB9 for ; Fri, 16 Aug 2013 04:07:10 -0700 (PDT) In-Reply-To: <20130806090944.GT22035@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============1458323504== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-VNO9osoITTXvlzdaMvcu" --=-VNO9osoITTXvlzdaMvcu Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, 2013-08-06 at 11:09 +0200, Daniel Vetter wrote: > On Mon, Aug 05, 2013 at 02:45:25PM +0300, Imre Deak wrote: > > At the moment any command line option handling done by tests will > > interfere with the option handling of the subtest interface. To fix thi= s > > add a new version of the subtest_init function accepting optional short > > and long command line options. Merge these together with the subtest > > interface's own long options and handle both together in the same > > getopt_long call. > >=20 > > Signed-off-by: Imre Deak >=20 > Hm, I've thought that getopt would filter the passed-in argv/argc arrays > and we could run a second getopt afterwards without too much interfence > (maybe we need to reset a few global getop state variables). But I'm not > sure since I've never tried it out. Am I wrong? Afaics getopt itself can't handle the long options (which we already have for subtests), it'll try to parse each character of the long option as a short one. We could still do the scanning twice by always using getopt_long, but there I don't like the fact that we would have to set opterr=3D0 and silently ignore invalid options. Also I thought that later we could add a check for clashing test case/subtest options and that's not possible by scanning twice. --Imre > -Daniel >=20 > > --- > > lib/drmtest.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++= +------- > > lib/drmtest.h | 6 +++++ > > 2 files changed, 81 insertions(+), 10 deletions(-) > >=20 > > diff --git a/lib/drmtest.c b/lib/drmtest.c > > index afbaa35..7a68091 100644 > > --- a/lib/drmtest.c > > +++ b/lib/drmtest.c > > @@ -657,7 +657,21 @@ void drmtest_stop_signal_helper(void) > > static bool list_subtests =3D false; > > static char *run_single_subtest =3D NULL; > > =20 > > -void drmtest_subtest_init(int argc, char **argv) > > +static void print_usage(const char *command_str, const char *help_str, > > + bool output_on_stderr) > > +{ > > + FILE *f =3D output_on_stderr ? stderr : stdout; > > + > > + fprintf(f, "Usage: %s [OPTIONS]\n" > > + " --list-subtests\n" > > + " --run-subtest \n" > > + "%s\n", command_str, help_str); > > +} > > + > > +int drmtest_subtest_init_parse_opts(int argc, char **argv, const char = *opts, > > + struct option *long_opts, > > + const char *help_str, > > + drmtest_opt_handler_t opt_handler) > > { > > int c, option_index =3D 0; > > static struct option long_options[] =3D { > > @@ -665,24 +679,75 @@ void drmtest_subtest_init(int argc, char **argv) > > {"run-subtest", 1, 0, 'r'}, > > {NULL, 0, 0, 0,} > > }; > > + struct option help_opt =3D > > + {"help", 0, 0, 'h'}; > > + const char *command_str; > > + char *short_opts; > > + struct option *combined_opts; > > + int extra_opts; > > + int all_opts; > > + int ret =3D 0; > > + > > + command_str =3D argv[0]; > > + if (strrchr(command_str, '/')) > > + command_str =3D strrchr(command_str, '/') + 1; > > + > > + all_opts =3D 0; > > + while (long_opts && long_opts[all_opts].name) > > + all_opts++; > > + extra_opts =3D all_opts; > > + if (help_str) > > + all_opts++; > > + all_opts +=3D ARRAY_SIZE(long_options); > > + > > + combined_opts =3D malloc(all_opts * sizeof(*combined_opts)); > > + memcpy(combined_opts, long_opts, extra_opts * sizeof(*combined_opts))= ; > > + if (help_str) { > > + combined_opts[extra_opts] =3D help_opt; > > + extra_opts++; > > + } > > + memcpy(&combined_opts[extra_opts], long_options, > > + ARRAY_SIZE(long_options) * sizeof(*combined_opts)); > > =20 > > - /* supress getopt errors about unknown options */ > > - opterr =3D 0; > > - /* restrict the option parsing to long option names to avoid collisio= ns > > - * with options the test declares */ > > - while((c =3D getopt_long(argc, argv, "", > > - long_options, &option_index)) !=3D -1) { > > + ret =3D asprintf(&short_opts, "%s%s", > > + opts ? opts : "", help_str ? "h" : ""); > > + assert(ret >=3D 0); > > + > > + while ((c =3D getopt_long(argc, argv, short_opts, combined_opts, > > + &option_index)) !=3D -1) { > > switch(c) { > > case 'l': > > - list_subtests =3D true; > > - goto out; > > + if (!run_single_subtest) > > + list_subtests =3D true; > > + break; > > case 'r': > > - run_single_subtest =3D strdup(optarg); > > + if (!list_subtests) > > + run_single_subtest =3D strdup(optarg); > > + break; > > + case '?': > > + case 'h': > > + print_usage(command_str, help_str, c =3D=3D '?'); > > + ret =3D c =3D=3D '?' ? -2 : -1; > > goto out; > > + default: > > + ret =3D opt_handler(c, option_index); > > + if (ret) > > + goto out; > > } > > } > > =20 > > out: > > + return ret; > > +} > > + > > +void drmtest_subtest_init(int argc, char **argv) > > +{ > > + /* supress getopt errors about unknown options */ > > + opterr =3D 0; > > + > > + (void)drmtest_subtest_init_parse_opts(argc, argv, NULL, NULL, NULL, > > + NULL); > > + > > /* reset opt parsing */ > > optind =3D 1; > > } > > diff --git a/lib/drmtest.h b/lib/drmtest.h > > index 172bed5..5ca7e2e 100644 > > --- a/lib/drmtest.h > > +++ b/lib/drmtest.h > > @@ -94,6 +94,12 @@ void drmtest_progress(const char *header, uint64_t i= , uint64_t total); > > =20 > > /* subtest infrastructure */ > > void drmtest_subtest_init(int argc, char **argv); > > +typedef int (*drmtest_opt_handler_t)(int opt, int opt_index); > > +struct option; > > +int drmtest_subtest_init_parse_opts(int argc, char **argv, const char = *opts, > > + struct option *long_opts, > > + const char *help_str, > > + drmtest_opt_handler_t opt_handler); > > bool drmtest_run_subtest(const char *subtest_name); > > bool drmtest_only_list_subtests(void); > > =20 > > --=20 > > 1.8.3.2 > >=20 > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >=20 --=-VNO9osoITTXvlzdaMvcu Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iQEcBAABAgAGBQJSDgfbAAoJEORIIAnNuWDFM3UH/iBeK4jGbXrxbabBealAKsHs frooaZqqDsTJXBCTya0g0lotVjQxSc/N738eSPuy6WV+mBO6UeqMxtEFkwRPbO4n BiVCSFQwmntJ3m4cDJWqB/7JWc+aTuROrjyk/WmpCnr3iSp89SjgOJ4ljNLilUZ6 oiOB6D/Lrmy5Q/fS8Tkm1ObNSKhd1/2Dm4BfDFgJxpPF34OGw9HFtNJg+A8zW8kg bQe3sS3ak/xcvVVVCtUw7sN3Q9muBKYoabcIJnFpwiuI+ZBQ2L3randJeeD9pXoI LTxUzWeAMd6JSukwkyVb1W1MYv5q94fjT6zoDRFDGRPgngbHdTCVHZ0IDWNwX9c= =JDaV -----END PGP SIGNATURE----- --=-VNO9osoITTXvlzdaMvcu-- --===============1458323504== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============1458323504==--