It would be really good to squash these patches introducing (and then modifying 
the previously introduced) new helper function prototypes into a single patch. 
This way it's hard to see the complete picture what functions were actually 
added into helper/linux.h


-Petri


> -----Original Message-----
> From: lng-odp [mailto:[email protected]] On Behalf Of
> Christophe Milard
> Sent: Wednesday, May 11, 2016 7:42 PM
> To: [email protected]; [email protected]; lng-
> [email protected]
> Subject: [lng-odp] [PATCHv6 12/38] helper: parsing the complete set of
> options
> 
> The odph_parse_options() function is given the ability to receive
> getopt command line description parameter from it caller, hence allowing
> the caller to have some command line parameter(s).
> The caller shall first call odph_parse_options() with its own parameter
> description as parameter.
> odph_parse_options() is then checking the complete set of options,
> issuing error message for unknown options (those being neither a caller's
> valid command line option or a helper valid command line option),
> and collecting the sementic of helper options.
> Then the caller shall parse the sementic of its own options,
> with the opterr variable set to zero (hence ignoring helper options).
> 
> Signed-off-by: Christophe Milard <[email protected]>
> ---
>  helper/include/odp/helper/linux.h         | 16 +++++++++++++++-
>  helper/linux.c                            | 24 +++++++++++++++++++-----
>  helper/test/odpthreads.c                  |  2 +-
>  test/validation/common/odp_cunit_common.c |  2 +-
>  4 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/helper/include/odp/helper/linux.h
> b/helper/include/odp/helper/linux.h
> index 9767af4..71c8027 100644
> --- a/helper/include/odp/helper/linux.h
> +++ b/helper/include/odp/helper/linux.h
> @@ -25,6 +25,7 @@ extern "C" {
>  #include <odp_api.h>
> 
>  #include <pthread.h>
> +#include <getopt.h>
>  #include <sys/types.h>
> 
>  /** Thread parameter for Linux pthreads and processes */
> @@ -241,15 +242,28 @@ int odph_merge_getopt_options(const char
> *shortopts1,
>   * Parse linux helper options
>   *
>   * Parse the command line options. Pick up options meant for the helper
> itself.
> + * If the caller is also having a set of option to parse, it should
> include
> + * their description here (shortopts desribes the short options and
> longopts
> + * describes the long options, as for getopt_long()).
> + * This function will issue errors on unknown arguments, so callers
> failing
> + * to pass their own command line options description here will see their
> + * options rejected.
> + * (the caller wants to set opterr to zero when parsing its own stuff
> + * with getopts to avoid reacting on helper's options).
>   *
>   * @param argc argument count
>   * @param argv argument values
> + * @param caller_shortopts caller's set of short options (string). or
> NULL.
> + * @param caller_longopts  caller's set of long options (getopt option
> array).
> + *                      or NULL.
>   *
>   * @return On success: 0
>   *      On failure: -1 (failure occurs only if a value passed for a
> helper
>   *                      option is invalid. callers cannot have own options)
>   */
> -int odph_parse_options(int argc, char *argv[]);
> +int odph_parse_options(int argc, char *argv[],
> +                    const char *caller_shortopts,
> +                    const struct option *caller_longopts);
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/helper/linux.c b/helper/linux.c
> index d1b7825..5fc09a1 100644
> --- a/helper/linux.c
> +++ b/helper/linux.c
> @@ -16,7 +16,6 @@
>  #include <stdlib.h>
>  #include <string.h>
>  #include <stdio.h>
> -#include <getopt.h>
> 
>  #include <odp_api.h>
>  #include <odp/helper/linux.h>
> @@ -576,27 +575,39 @@ int odph_merge_getopt_options(const char
> *shortopts1,
>  /*
>   * Parse command line options to extract options affecting helpers.
>   */
> -int odph_parse_options(int argc, char *argv[])
> +int odph_parse_options(int argc, char *argv[],
> +                    const char *caller_shortopts,
> +                    const struct option *caller_longopts)
>  {
>       int c;
> +     char *shortopts;
> +     struct option *longopts;
> 
> -     static struct option long_options[] = {
> +     static struct option helper_long_options[] = {
>               /* These options set a flag. */
>               {"odph_proc",   no_argument, &helper_options.proc, 1},
>               {"odph_thread", no_argument, &helper_options.thrd, 1},
>               {0, 0, 0, 0}
>               };
> 
> +     static char *helper_short_options = "";
> +
>       /* defaults: */
>       helper_options.proc = false;
>       helper_options.thrd = false;
> 
> +     /* merge caller's command line options descriptions with helper's:
> */
> +     if (odph_merge_getopt_options(caller_shortopts,
> helper_short_options,
> +                                   caller_longopts, helper_long_options,
> +                                   &shortopts, &longopts) < 0)
> +             return -1;
> +
>       while (1) {
>               /* getopt_long stores the option index here. */
>               int option_index = 0;
> 
> -             c = getopt_long (argc, argv, "",
> -                              long_options, &option_index);
> +             c = getopt_long (argc, argv,
> +                              shortopts, longopts, &option_index);
> 
>               /* Detect the end of the options. */
>               if (c == -1)
> @@ -605,5 +616,8 @@ int odph_parse_options(int argc, char *argv[])
> 
>       optind = 0; /* caller expects this to be zero if it parses too*/
> 
> +     free(shortopts);
> +     free(longopts);
> +
>       return 0;
>  }
> diff --git a/helper/test/odpthreads.c b/helper/test/odpthreads.c
> index 369da62..bba4fa5 100644
> --- a/helper/test/odpthreads.c
> +++ b/helper/test/odpthreads.c
> @@ -39,7 +39,7 @@ int main(int argc, char *argv[])
>       char cpumaskstr[ODP_CPUMASK_STR_SIZE];
> 
>       /* let helper collect its own arguments (e.g. --odph_proc) */
> -     odph_parse_options(argc, argv);
> +     odph_parse_options(argc, argv, NULL, NULL);
> 
>       if (odp_init_global(&instance, NULL, NULL)) {
>               LOG_ERR("Error: ODP global init failed.\n");
> diff --git a/test/validation/common/odp_cunit_common.c
> b/test/validation/common/odp_cunit_common.c
> index be23c6b..7df9aa6 100644
> --- a/test/validation/common/odp_cunit_common.c
> +++ b/test/validation/common/odp_cunit_common.c
> @@ -354,5 +354,5 @@ int odp_cunit_register(odp_suiteinfo_t testsuites[])
>   */
>  int odp_cunit_parse_options(int argc, char *argv[])
>  {
> -     return odph_parse_options(argc, argv);
> +     return odph_parse_options(argc, argv, NULL, NULL);
>  }
> --
> 2.5.0
> 
> _______________________________________________
> lng-odp mailing list
> [email protected]
> https://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to