On Thu, Oct 15, 2015 at 04:22:53PM +0000, Don Provan wrote:
> Looks perfect. Thanks!

Thanks! It's my pleasure. :-)

Best wishes,
Tiwei Bie

> -don
> 
> -----Original Message-----
> From: Tiwei Bie [mailto:btw at mail.ustc.edu.cn] 
> Sent: Thursday, October 15, 2015 4:46 AM
> To: Don Provan <dprovan at bivio.net>; bruce.richardson at intel.com; dev at 
> dpdk.org
> Subject: [PATCH] eal: don't reset getopt lib
> 
> Someone may need to call rte_eal_init() with a fake argc/argv array in the 
> middle of using getopt() to parse its own unrelated argc/argv parameters. So 
> getopt lib shouldn't be reset by rte_eal_init().
> 
> Now eal will always save optind, optarg and optopt (and optreset on
> FreeBSD) at the beginning, initialize optind (and optreset on FreeBSD) to 1 
> before calling getopt_long(), then restore all values after.
> 
> Suggested-by: Don Provan <dprovan at bivio.net>
> Suggested-by: Bruce Richardson <bruce.richardson at intel.com>
> Signed-off-by: Tiwei Bie <btw at mail.ustc.edu.cn>
> ---
>  lib/librte_eal/bsdapp/eal/eal.c   | 59 +++++++++++++++++++++++++++------
>  lib/librte_eal/linuxapp/eal/eal.c | 69 
> ++++++++++++++++++++++++++++++---------
>  2 files changed, 102 insertions(+), 26 deletions(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c 
> b/lib/librte_eal/bsdapp/eal/eal.c index 1b6f705..bd09377 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -312,8 +312,20 @@ eal_log_level_parse(int argc, char **argv)
>       int opt;
>       char **argvopt;
>       int option_index;
> +     int old_optind;
> +     int old_optopt;
> +     int old_optreset;
> +     char *old_optarg;
> +
> +     /* save getopt lib */
> +     old_optind = optind;
> +     old_optopt = optopt;
> +     old_optreset = optreset;
> +     old_optarg = optarg;
>  
>       argvopt = argv;
> +     optind = 1;
> +     optreset = 1;
>  
>       eal_reset_internal_config(&internal_config);
>  
> @@ -334,7 +346,11 @@ eal_log_level_parse(int argc, char **argv)
>                       break;
>       }
>  
> -     optind = 0; /* reset getopt lib */
> +     /* restore getopt lib */
> +     optind = old_optind;
> +     optopt = old_optopt;
> +     optreset = old_optreset;
> +     optarg = old_optarg;
>  }
>  
>  /* Parse the argument given in the command line of the application */ @@ 
> -345,25 +361,37 @@ eal_parse_args(int argc, char **argv)
>       char **argvopt;
>       int option_index;
>       char *prgname = argv[0];
> +     int old_optind;
> +     int old_optopt;
> +     int old_optreset;
> +     char *old_optarg;
> +
> +     /* save getopt lib */
> +     old_optind = optind;
> +     old_optopt = optopt;
> +     old_optreset = optreset;
> +     old_optarg = optarg;
>  
>       argvopt = argv;
> +     optind = 1;
> +     optreset = 1;
>  
>       while ((opt = getopt_long(argc, argvopt, eal_short_options,
>                                 eal_long_options, &option_index)) != EOF) {
>  
> -             int ret;
> -
>               /* getopt is not happy, stop right now */
>               if (opt == '?') {
>                       eal_usage(prgname);
> -                     return -1;
> +                     ret = -1;
> +                     goto out;
>               }
>  
>               ret = eal_parse_common_option(opt, optarg, &internal_config);
>               /* common parser is not happy */
>               if (ret < 0) {
>                       eal_usage(prgname);
> -                     return -1;
> +                     ret = -1;
> +                     goto out;
>               }
>               /* common parser handled this option */
>               if (ret == 0)
> @@ -387,23 +415,34 @@ eal_parse_args(int argc, char **argv)
>                                       "on FreeBSD\n", opt);
>                       }
>                       eal_usage(prgname);
> -                     return -1;
> +                     ret = -1;
> +                     goto out;
>               }
>       }
>  
> -     if (eal_adjust_config(&internal_config) != 0)
> -             return -1;
> +     if (eal_adjust_config(&internal_config) != 0) {
> +             ret = -1;
> +             goto out;
> +     }
>  
>       /* sanity checks */
>       if (eal_check_common_options(&internal_config) != 0) {
>               eal_usage(prgname);
> -             return -1;
> +             ret = -1;
> +             goto out;
>       }
>  
>       if (optind >= 0)
>               argv[optind-1] = prgname;
>       ret = optind-1;
> -     optind = 0; /* reset getopt lib */
> +
> +out:
> +     /* restore getopt lib */
> +     optind = old_optind;
> +     optopt = old_optopt;
> +     optreset = old_optreset;
> +     optarg = old_optarg;
> +
>       return ret;
>  }
>  
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c 
> b/lib/librte_eal/linuxapp/eal/eal.c
> index 33e1067..4796030 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -505,8 +505,17 @@ eal_log_level_parse(int argc, char **argv)
>       int opt;
>       char **argvopt;
>       int option_index;
> +     int old_optind;
> +     int old_optopt;
> +     char *old_optarg;
> +
> +     /* save getopt lib */
> +     old_optind = optind;
> +     old_optopt = optopt;
> +     old_optarg = optarg;
>  
>       argvopt = argv;
> +     optind = 1;
>  
>       eal_reset_internal_config(&internal_config);
>  
> @@ -527,7 +536,10 @@ eal_log_level_parse(int argc, char **argv)
>                       break;
>       }
>  
> -     optind = 0; /* reset getopt lib */
> +     /* restore getopt lib */
> +     optind = old_optind;
> +     optopt = old_optopt;
> +     optarg = old_optarg;
>  }
>  
>  /* Parse the argument given in the command line of the application */ @@ 
> -539,25 +551,34 @@ eal_parse_args(int argc, char **argv)
>       int option_index;
>       char *prgname = argv[0];
>       struct shared_driver *solib;
> +     int old_optind;
> +     int old_optopt;
> +     char *old_optarg;
> +
> +     /* save getopt lib */
> +     old_optind = optind;
> +     old_optopt = optopt;
> +     old_optarg = optarg;
>  
>       argvopt = argv;
> +     optind = 1;
>  
>       while ((opt = getopt_long(argc, argvopt, eal_short_options,
>                                 eal_long_options, &option_index)) != EOF) {
>  
> -             int ret;
> -
>               /* getopt is not happy, stop right now */
>               if (opt == '?') {
>                       eal_usage(prgname);
> -                     return -1;
> +                     ret = -1;
> +                     goto out;
>               }
>  
>               ret = eal_parse_common_option(opt, optarg, &internal_config);
>               /* common parser is not happy */
>               if (ret < 0) {
>                       eal_usage(prgname);
> -                     return -1;
> +                     ret = -1;
> +                     goto out;
>               }
>               /* common parser handled this option */
>               if (ret == 0)
> @@ -573,7 +594,8 @@ eal_parse_args(int argc, char **argv)
>                       solib = malloc(sizeof(*solib));
>                       if (solib == NULL) {
>                               RTE_LOG(ERR, EAL, "malloc(solib) failed\n");
> -                             return -1;
> +                             ret = -1;
> +                             goto out;
>                       }
>                       memset(solib, 0, sizeof(*solib));
>                       strncpy(solib->name, optarg, PATH_MAX-1); @@ -589,7 
> +611,8 @@ eal_parse_args(int argc, char **argv)
>                       RTE_LOG(ERR, EAL, "Can't support DPDK app "
>                               "running on Dom0, please configure"
>                               " RTE_LIBRTE_XEN_DOM0=y\n");
> -                     return -1;
> +                     ret = -1;
> +                     goto out;
>  #endif
>                       break;
>  
> @@ -606,7 +629,8 @@ eal_parse_args(int argc, char **argv)
>                               RTE_LOG(ERR, EAL, "invalid parameters for --"
>                                               OPT_SOCKET_MEM "\n");
>                               eal_usage(prgname);
> -                             return -1;
> +                             ret = -1;
> +                             goto out;
>                       }
>                       break;
>  
> @@ -615,7 +639,8 @@ eal_parse_args(int argc, char **argv)
>                               RTE_LOG(ERR, EAL, "invalid parameter for --"
>                                               OPT_BASE_VIRTADDR "\n");
>                               eal_usage(prgname);
> -                             return -1;
> +                             ret = -1;
> +                             goto out;
>                       }
>                       break;
>  
> @@ -624,7 +649,8 @@ eal_parse_args(int argc, char **argv)
>                               RTE_LOG(ERR, EAL, "invalid parameters for --"
>                                               OPT_VFIO_INTR "\n");
>                               eal_usage(prgname);
> -                             return -1;
> +                             ret = -1;
> +                             goto out;
>                       }
>                       break;
>  
> @@ -646,17 +672,21 @@ eal_parse_args(int argc, char **argv)
>                                       "on Linux\n", opt);
>                       }
>                       eal_usage(prgname);
> -                     return -1;
> +                     ret = -1;
> +                     goto out;
>               }
>       }
>  
> -     if (eal_adjust_config(&internal_config) != 0)
> -             return -1;
> +     if (eal_adjust_config(&internal_config) != 0) {
> +             ret = -1;
> +             goto out;
> +     }
>  
>       /* sanity checks */
>       if (eal_check_common_options(&internal_config) != 0) {
>               eal_usage(prgname);
> -             return -1;
> +             ret = -1;
> +             goto out;
>       }
>  
>       /* --xen-dom0 doesn't make sense with --socket-mem */ @@ -664,13 
> +694,20 @@ eal_parse_args(int argc, char **argv)
>               RTE_LOG(ERR, EAL, "Options --"OPT_SOCKET_MEM" cannot be 
> specified "
>                       "together with --"OPT_XEN_DOM0"\n");
>               eal_usage(prgname);
> -             return -1;
> +             ret = -1;
> +             goto out;
>       }
>  
>       if (optind >= 0)
>               argv[optind-1] = prgname;
>       ret = optind-1;
> -     optind = 0; /* reset getopt lib */
> +
> +out:
> +     /* restore getopt lib */
> +     optind = old_optind;
> +     optopt = old_optopt;
> +     optarg = old_optarg;
> +
>       return ret;
>  }
>  
> --
> 2.1.2
> 
> 

Reply via email to