From: Tiwei Bie <btw@mail.ustc.edu.cn>
To: Don Provan <dprovan@bivio.net>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [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 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 ` [dpdk-dev] [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).