From: Tiwei Bie <btw@mail.ustc.edu.cn>
To: Don Provan <dprovan@bivio.net>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [PATCH] eal: don't reset getopt lib
Date: Fri, 16 Oct 2015 10:25:29 +0800 [thread overview]
Message-ID: <20151016022529.GA9234@dell> (raw)
In-Reply-To: <CY1PR0101MB098739E9058418FC61EDDB56A03E0@CY1PR0101MB0987.prod.exchangelabs.com>
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@mail.ustc.edu.cn]
> Sent: Thursday, October 15, 2015 4:46 AM
> To: Don Provan <dprovan@bivio.net>; bruce.richardson@intel.com; dev@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@bivio.net>
> Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
> Signed-off-by: Tiwei Bie <btw@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
>
>
next prev parent reply other threads:[~2015-10-16 2:25 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-15 11:46 [PATCH] eal: don't reset getopt lib Tiwei Bie
2015-10-15 16:22 ` Don Provan
2015-10-16 2:25 ` Tiwei Bie [this message]
2015-10-19 10:36 ` Bruce Richardson
2015-10-19 13:15 ` Tiwei Bie
2015-10-19 13:13 ` [PATCH v2] " Tiwei Bie
2015-10-19 13:16 ` Bruce Richardson
2015-10-21 5:33 ` David Marchand
2015-10-21 6:17 ` Tiwei Bie
2015-11-04 22:21 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151016022529.GA9234@dell \
--to=btw@mail.ustc.edu.cn \
--cc=dev@dpdk.org \
--cc=dprovan@bivio.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.