* [dpdk-dev] [PATCH] Found a bug related to getopt() in eal/bsd module @ 2015-10-13 8:54 Tiwei Bie 2015-10-13 8:54 ` [dpdk-dev] [PATCH] eal/bsd: reinitialize optind and optreset to 1 Tiwei Bie 0 siblings, 1 reply; 9+ messages in thread From: Tiwei Bie @ 2015-10-13 8:54 UTC (permalink / raw) To: dev I found a bug when trying to make my DPDK application work on FreeBSD. The variable optind must be reinitialized to 1 on FreeBSD to skip over argv[0]. Because getopt() on FreeBSD will return -1 when it meets an argument which doesn't start with '-'. This behaviour is implemented by the 13-17 lines: 01 /* 02 * getopt -- 03 * Parse argc/argv argument vector. 04 */ 05 int 06 getopt(int nargc, char * const nargv[], const char *ostr) 07 { 08 static char *place = EMSG; /* option letter processing */ 09 char *oli; /* option letter list index */ 10 11 if (optreset || *place == 0) { /* update scanning pointer */ 12 optreset = 0; 13 place = nargv[optind]; 14 if (optind >= nargc || *place++ != '-') { 15 /* Argument is absent or is not an option */ 16 place = EMSG; 17 return (-1); 18 } 19 ...... 20 } 21 ...... 22 } The variable optreset is also provided on FreeBSD to indicate the additional set of calls to getopt(). So, also reinitialize it to 1. References: 1. https://svnweb.freebsd.org/base/head/lib/libc/stdlib/getopt.c?view=markup#l70 2. https://www.freebsd.org/cgi/man.cgi?query=getopt&apropos=0&sektion=3&manpath=FreeBSD+11-current&arch=default&format=html Tiwei Bie (1): eal/bsd: reinitialize optind and optreset to 1 lib/librte_eal/bsdapp/eal/eal.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) -- 2.6.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [dpdk-dev] [PATCH] eal/bsd: reinitialize optind and optreset to 1 2015-10-13 8:54 [dpdk-dev] [PATCH] Found a bug related to getopt() in eal/bsd module Tiwei Bie @ 2015-10-13 8:54 ` Tiwei Bie 2015-10-13 14:58 ` Bruce Richardson 2015-10-13 17:14 ` Don Provan 0 siblings, 2 replies; 9+ messages in thread From: Tiwei Bie @ 2015-10-13 8:54 UTC (permalink / raw) To: dev The variable optind must be reinitialized to 1 in order to skip over argv[0] on FreeBSD. Because getopt() on FreeBSD will return -1 when it meets an argument which doesn't start with '-'. The variable optreset is provided on FreeBSD to indicate the additional set of calls to getopt(). So, also reinitialize it to 1. Signed-off-by: Tiwei Bie <btw@mail.ustc.edu.cn> --- lib/librte_eal/bsdapp/eal/eal.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c index 1b6f705..35feaee 100644 --- a/lib/librte_eal/bsdapp/eal/eal.c +++ b/lib/librte_eal/bsdapp/eal/eal.c @@ -334,7 +334,8 @@ eal_log_level_parse(int argc, char **argv) break; } - optind = 0; /* reset getopt lib */ + optind = 1; /* reset getopt lib */ + optreset = 1; } /* Parse the argument given in the command line of the application */ @@ -403,7 +404,8 @@ eal_parse_args(int argc, char **argv) if (optind >= 0) argv[optind-1] = prgname; ret = optind-1; - optind = 0; /* reset getopt lib */ + optind = 1; /* reset getopt lib */ + optreset = 1; return ret; } -- 2.6.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] eal/bsd: reinitialize optind and optreset to 1 2015-10-13 8:54 ` [dpdk-dev] [PATCH] eal/bsd: reinitialize optind and optreset to 1 Tiwei Bie @ 2015-10-13 14:58 ` Bruce Richardson 2015-10-13 17:14 ` Don Provan 1 sibling, 0 replies; 9+ messages in thread From: Bruce Richardson @ 2015-10-13 14:58 UTC (permalink / raw) To: Tiwei Bie; +Cc: dev On Tue, Oct 13, 2015 at 04:54:06PM +0800, Tiwei Bie wrote: > The variable optind must be reinitialized to 1 in order to skip over > argv[0] on FreeBSD. Because getopt() on FreeBSD will return -1 when > it meets an argument which doesn't start with '-'. > > The variable optreset is provided on FreeBSD to indicate the additional > set of calls to getopt(). So, also reinitialize it to 1. > > Signed-off-by: Tiwei Bie <btw@mail.ustc.edu.cn> Acked-by: Bruce Richardson <bruce.richardson@intel.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] eal/bsd: reinitialize optind and optreset to 1 2015-10-13 8:54 ` [dpdk-dev] [PATCH] eal/bsd: reinitialize optind and optreset to 1 Tiwei Bie 2015-10-13 14:58 ` Bruce Richardson @ 2015-10-13 17:14 ` Don Provan [not found] ` <20151014022843.GA26774@dell> 1 sibling, 1 reply; 9+ messages in thread From: Don Provan @ 2015-10-13 17:14 UTC (permalink / raw) To: Tiwei Bie, dev Actually, this is a good opportunity to fix a bug that's been in this code forever: it shouldn't be resetting optind to some arbitrary value: it should be saving optind (and optarg and optopt) at the beginning, initializing optind to 1 before calling getopt_long(), then restoring all the values after. (And, from what you're saying, optreset should be handled the same as optind.) This avoids broken behavior if rte_eal_init() is called by code that's in the middle of using getopt() to parse its own unrelated argc/argv parameters. -don provan dprovan@bivio.net -----Original Message----- From: Tiwei Bie [mailto:btw@mail.ustc.edu.cn] Sent: Tuesday, October 13, 2015 1:54 AM To: dev@dpdk.org Subject: [dpdk-dev] [PATCH] eal/bsd: reinitialize optind and optreset to 1 The variable optind must be reinitialized to 1 in order to skip over argv[0] on FreeBSD. Because getopt() on FreeBSD will return -1 when it meets an argument which doesn't start with '-'. The variable optreset is provided on FreeBSD to indicate the additional set of calls to getopt(). So, also reinitialize it to 1. Signed-off-by: Tiwei Bie <btw@mail.ustc.edu.cn> --- lib/librte_eal/bsdapp/eal/eal.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c index 1b6f705..35feaee 100644 --- a/lib/librte_eal/bsdapp/eal/eal.c +++ b/lib/librte_eal/bsdapp/eal/eal.c @@ -334,7 +334,8 @@ eal_log_level_parse(int argc, char **argv) break; } - optind = 0; /* reset getopt lib */ + optind = 1; /* reset getopt lib */ + optreset = 1; } /* Parse the argument given in the command line of the application */ @@ -403,7 +404,8 @@ eal_parse_args(int argc, char **argv) if (optind >= 0) argv[optind-1] = prgname; ret = optind-1; - optind = 0; /* reset getopt lib */ + optind = 1; /* reset getopt lib */ + optreset = 1; return ret; } -- 2.6.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <20151014022843.GA26774@dell>]
* Re: [dpdk-dev] [PATCH] eal/bsd: reinitialize optind and optreset to 1 [not found] ` <20151014022843.GA26774@dell> @ 2015-10-14 9:31 ` Bruce Richardson 2015-10-14 10:19 ` Tiwei Bie 0 siblings, 1 reply; 9+ messages in thread From: Bruce Richardson @ 2015-10-14 9:31 UTC (permalink / raw) To: Tiwei Bie; +Cc: dev, Don Provan On Wed, Oct 14, 2015 at 10:28:44AM +0800, Tiwei Bie wrote: > On Tue, Oct 13, 2015 at 05:14:38PM +0000, Don Provan wrote: > > Actually, this is a good opportunity to fix a bug that's been in this code forever: it shouldn't be resetting optind to some arbitrary value: it should be saving optind (and optarg and optopt) at the beginning, initializing optind to 1 before calling getopt_long(), then restoring all the values after. (And, from what you're saying, optreset should be handled the same as optind.) > > > > It is designed to have DPDK's parameters specified in the front of the > cmd line and terminated by '--'. Or at least, you should put DPDK's > parameters together and terminate them by '--'. And 1 or 0 are not some > arbitrary values. They are used to put the index back to the beginning > of the new argv[] array. > > > This avoids broken behavior if rte_eal_init() is called by code that's in the middle of using getopt() to parse its own unrelated argc/argv parameters. > > > > We shouldn't mix up DPDK's parameters and application's parameters. > And we should group them using '--'. > > Best, > Tiwei Bie While true, that does not prevent us from implementing Don's suggestion, as it should fix the bug you are looking at with your original patch, and also allow additional use-cases for applications at no extra cost. /Bruce > > > -don provan > > dprovan@bivio.net > > > > -----Original Message----- > > From: Tiwei Bie [mailto:btw@mail.ustc.edu.cn] > > Sent: Tuesday, October 13, 2015 1:54 AM > > To: dev@dpdk.org > > Subject: [dpdk-dev] [PATCH] eal/bsd: reinitialize optind and optreset to 1 > > > > The variable optind must be reinitialized to 1 in order to skip over argv[0] on FreeBSD. Because getopt() on FreeBSD will return -1 when it meets an argument which doesn't start with '-'. > > > > The variable optreset is provided on FreeBSD to indicate the additional set of calls to getopt(). So, also reinitialize it to 1. > > > > Signed-off-by: Tiwei Bie <btw@mail.ustc.edu.cn> > > --- > > lib/librte_eal/bsdapp/eal/eal.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c index 1b6f705..35feaee 100644 > > --- a/lib/librte_eal/bsdapp/eal/eal.c > > +++ b/lib/librte_eal/bsdapp/eal/eal.c > > @@ -334,7 +334,8 @@ eal_log_level_parse(int argc, char **argv) > > break; > > } > > > > - optind = 0; /* reset getopt lib */ > > + optind = 1; /* reset getopt lib */ > > + optreset = 1; > > } > > > > /* Parse the argument given in the command line of the application */ @@ -403,7 +404,8 @@ eal_parse_args(int argc, char **argv) > > if (optind >= 0) > > argv[optind-1] = prgname; > > ret = optind-1; > > - optind = 0; /* reset getopt lib */ > > + optind = 1; /* reset getopt lib */ > > + optreset = 1; > > return ret; > > } > > > > -- > > 2.6.0 > > > > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] eal/bsd: reinitialize optind and optreset to 1 2015-10-14 9:31 ` Bruce Richardson @ 2015-10-14 10:19 ` Tiwei Bie 2015-10-14 11:28 ` Tiwei Bie 0 siblings, 1 reply; 9+ messages in thread From: Tiwei Bie @ 2015-10-14 10:19 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev, Don Provan On Wed, Oct 14, 2015 at 10:31:28AM +0100, Bruce Richardson wrote: > On Wed, Oct 14, 2015 at 10:28:44AM +0800, Tiwei Bie wrote: > > On Tue, Oct 13, 2015 at 05:14:38PM +0000, Don Provan wrote: > > > Actually, this is a good opportunity to fix a bug that's been in this code forever: it shouldn't be resetting optind to some arbitrary value: it should be saving optind (and optarg and optopt) at the beginning, initializing optind to 1 before calling getopt_long(), then restoring all the values after. (And, from what you're saying, optreset should be handled the same as optind.) > > > > > > > It is designed to have DPDK's parameters specified in the front of the > > cmd line and terminated by '--'. Or at least, you should put DPDK's > > parameters together and terminate them by '--'. And 1 or 0 are not some > > arbitrary values. They are used to put the index back to the beginning > > of the new argv[] array. > > > > > This avoids broken behavior if rte_eal_init() is called by code that's in the middle of using getopt() to parse its own unrelated argc/argv parameters. > > > > > > > We shouldn't mix up DPDK's parameters and application's parameters. > > And we should group them using '--'. > > > > Best, > > Tiwei Bie > > While true, that does not prevent us from implementing Don's suggestion, as it > should fix the bug you are looking at with your original patch, and also allow > additional use-cases for applications at no extra cost. > As I understand it, what Don wants is something like this: int main(int argc, char **argv) { int ret; int ch; while ((ch = getopt(argc, argv, "whateveroptions:d")) != -1) { switch (ch) { case 'd': /* rte_eal_init() will be called */ ret = dpdk_init(argc, argv); argc -= ret; argv += ret; break; case 'w': ...... } } argc -= optind; argv += optind; ...... } static int dpdk_init(int argc, char **argv) { int ret; ret = rte_eal_init(argc, argv); if (ret < 0) FATAL_ERROR("Could not initialise EAL (%d)", ret); ...... return (ret); } And the current code should work correctly if DPDK's parameters are put together and terminated by '--': $ ./demo -whatever -d -c f -n 2 -- -option -s hello The only limitation is that the return value of rte_eal_init() must be returned to the code which is using getopt() to parse argc/argv. And I'm very willing to rework my patch to get rid of this limitation. Best regards, Tiwei Bie > /Bruce > > > -----Original Message----- > > From: Tiwei Bie [mailto:btw@mail.ustc.edu.cn] > > Sent: Wednesday, October 14, 2015 6:48 AM > > To: Zhu, Heqing > > Cc: Richardson, Bruce; O'Driscoll, Tim; Ananyev, Konstantin > > Subject: Re: [dpdk-dev] [PATCH] eal/bsd: reinitialize optind and optreset > > to 1 > > > > On Wed, Oct 14, 2015 at 05:26:17AM +0000, Zhu, Heqing wrote: > > > > > > Bruce, thank you. Tiwei will join DPDK team in 2016. :) > > > > > > @Tiwei, Bruce/Konstantin are the DPDK tech leads, both work and live in > > Ireland. > > > > > > > Cool! It's my great honor to join DPDK team! Thank you very much! :-) > > > > Best regards, > > Tiwei Bie > > > > > > > > > > > -----Original Message----- > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson > > > Sent: Tuesday, October 13, 2015 10:59 PM > > > To: Tiwei Bie > > > Cc: dev@dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH] eal/bsd: reinitialize optind and > > > optreset to 1 > > > > > > On Tue, Oct 13, 2015 at 04:54:06PM +0800, Tiwei Bie wrote: > > > > The variable optind must be reinitialized to 1 in order to skip over > > > > argv[0] on FreeBSD. Because getopt() on FreeBSD will return -1 when > > > > it meets an argument which doesn't start with '-'. > > > > > > > > The variable optreset is provided on FreeBSD to indicate the > > > > additional set of calls to getopt(). So, also reinitialize it to 1. > > > > > > > > Signed-off-by: Tiwei Bie <btw@mail.ustc.edu.cn> > > > > > > Acked-by: Bruce Richardson <bruce.richardson@intel.com> > > > > > /Bruce > > > > > > -don provan > > > dprovan@bivio.net > > > > > > -----Original Message----- > > > From: Tiwei Bie [mailto:btw@mail.ustc.edu.cn] > > > Sent: Tuesday, October 13, 2015 1:54 AM > > > To: dev@dpdk.org > > > Subject: [dpdk-dev] [PATCH] eal/bsd: reinitialize optind and optreset to 1 > > > > > > The variable optind must be reinitialized to 1 in order to skip over argv[0] on FreeBSD. Because getopt() on FreeBSD will return -1 when it meets an argument which doesn't start with '-'. > > > > > > The variable optreset is provided on FreeBSD to indicate the additional set of calls to getopt(). So, also reinitialize it to 1. > > > > > > Signed-off-by: Tiwei Bie <btw@mail.ustc.edu.cn> > > > --- > > > lib/librte_eal/bsdapp/eal/eal.c | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c index 1b6f705..35feaee 100644 > > > --- a/lib/librte_eal/bsdapp/eal/eal.c > > > +++ b/lib/librte_eal/bsdapp/eal/eal.c > > > @@ -334,7 +334,8 @@ eal_log_level_parse(int argc, char **argv) > > > break; > > > } > > > > > > - optind = 0; /* reset getopt lib */ > > > + optind = 1; /* reset getopt lib */ > > > + optreset = 1; > > > } > > > > > > /* Parse the argument given in the command line of the application */ @@ -403,7 +404,8 @@ eal_parse_args(int argc, char **argv) > > > if (optind >= 0) > > > argv[optind-1] = prgname; > > > ret = optind-1; > > > - optind = 0; /* reset getopt lib */ > > > + optind = 1; /* reset getopt lib */ > > > + optreset = 1; > > > return ret; > > > } > > > > > > -- > > > 2.6.0 > > > > > > > > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] eal/bsd: reinitialize optind and optreset to 1 2015-10-14 10:19 ` Tiwei Bie @ 2015-10-14 11:28 ` Tiwei Bie 2015-10-14 17:54 ` Don Provan 0 siblings, 1 reply; 9+ messages in thread From: Tiwei Bie @ 2015-10-14 11:28 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev, Don Provan On Wed, Oct 14, 2015 at 06:19:33PM +0800, Tiwei Bie wrote: > On Wed, Oct 14, 2015 at 10:31:28AM +0100, Bruce Richardson wrote: > > On Wed, Oct 14, 2015 at 10:28:44AM +0800, Tiwei Bie wrote: > > > On Tue, Oct 13, 2015 at 05:14:38PM +0000, Don Provan wrote: > > > > Actually, this is a good opportunity to fix a bug that's been in this code forever: it shouldn't be resetting optind to some arbitrary value: it should be saving optind (and optarg and optopt) at the beginning, initializing optind to 1 before calling getopt_long(), then restoring all the values after. (And, from what you're saying, optreset should be handled the same as optind.) > > > > > > > > > > It is designed to have DPDK's parameters specified in the front of the > > > cmd line and terminated by '--'. Or at least, you should put DPDK's > > > parameters together and terminate them by '--'. And 1 or 0 are not some > > > arbitrary values. They are used to put the index back to the beginning > > > of the new argv[] array. > > > > > > > This avoids broken behavior if rte_eal_init() is called by code that's in the middle of using getopt() to parse its own unrelated argc/argv parameters. > > > > > > > > > > We shouldn't mix up DPDK's parameters and application's parameters. > > > And we should group them using '--'. > > > > > > Best, > > > Tiwei Bie > > > > While true, that does not prevent us from implementing Don's suggestion, as it > > should fix the bug you are looking at with your original patch, and also allow > > additional use-cases for applications at no extra cost. > > > > As I understand it, what Don wants is something like this: > > int main(int argc, char **argv) > { > int ret; > int ch; > > while ((ch = getopt(argc, argv, "whateveroptions:d")) != -1) { > switch (ch) { > case 'd': > /* rte_eal_init() will be called */ > ret = dpdk_init(argc, argv); > argc -= ret; > argv += ret; > break; > case 'w': > ...... > } > } > > argc -= optind; > argv += optind; > > ...... > } > > static int > dpdk_init(int argc, char **argv) > { > int ret; > > ret = rte_eal_init(argc, argv); > if (ret < 0) > FATAL_ERROR("Could not initialise EAL (%d)", ret); > > ...... > > return (ret); > } > > And the current code should work correctly if DPDK's parameters are > put together and terminated by '--': > > $ ./demo -whatever -d -c f -n 2 -- -option -s hello > eal_log_level_parse() needs to be reworked to make the code work correctly. > The only limitation is that the return value of rte_eal_init() must > be returned to the code which is using getopt() to parse argc/argv. > > And I'm very willing to rework my patch to get rid of this limitation. > I'm considering updating optind to make it point to the option after '--' before eal_parse_args() returns, and eal_parse_args() will return 0 to avoid breaking the current applications which use the return value of rte_eal_init() to update argc/argv. Any comments? Thanks! :-) Best regards, Tiwei Bie > Best regards, > Tiwei Bie > > > /Bruce > > > > > > > > > -don provan > > > > dprovan@bivio.net > > > > > > > > -----Original Message----- > > > > From: Tiwei Bie [mailto:btw@mail.ustc.edu.cn] > > > > Sent: Tuesday, October 13, 2015 1:54 AM > > > > To: dev@dpdk.org > > > > Subject: [dpdk-dev] [PATCH] eal/bsd: reinitialize optind and optreset to 1 > > > > > > > > The variable optind must be reinitialized to 1 in order to skip over argv[0] on FreeBSD. Because getopt() on FreeBSD will return -1 when it meets an argument which doesn't start with '-'. > > > > > > > > The variable optreset is provided on FreeBSD to indicate the additional set of calls to getopt(). So, also reinitialize it to 1. > > > > > > > > Signed-off-by: Tiwei Bie <btw@mail.ustc.edu.cn> > > > > --- > > > > lib/librte_eal/bsdapp/eal/eal.c | 6 ++++-- > > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c index 1b6f705..35feaee 100644 > > > > --- a/lib/librte_eal/bsdapp/eal/eal.c > > > > +++ b/lib/librte_eal/bsdapp/eal/eal.c > > > > @@ -334,7 +334,8 @@ eal_log_level_parse(int argc, char **argv) > > > > break; > > > > } > > > > > > > > - optind = 0; /* reset getopt lib */ > > > > + optind = 1; /* reset getopt lib */ > > > > + optreset = 1; > > > > } > > > > > > > > /* Parse the argument given in the command line of the application */ @@ -403,7 +404,8 @@ eal_parse_args(int argc, char **argv) > > > > if (optind >= 0) > > > > argv[optind-1] = prgname; > > > > ret = optind-1; > > > > - optind = 0; /* reset getopt lib */ > > > > + optind = 1; /* reset getopt lib */ > > > > + optreset = 1; > > > > return ret; > > > > } > > > > > > > > -- > > > > 2.6.0 > > > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] eal/bsd: reinitialize optind and optreset to 1 2015-10-14 11:28 ` Tiwei Bie @ 2015-10-14 17:54 ` Don Provan 2015-10-15 1:40 ` Tiwei Bie 0 siblings, 1 reply; 9+ messages in thread From: Don Provan @ 2015-10-14 17:54 UTC (permalink / raw) To: Tiwei Bie, Bruce Richardson; +Cc: dev > > On Wed, Oct 14, 2015 at 10:28:44AM +0800, Tiwei Bie wrote: > > > It is designed to have DPDK's parameters specified in the front of the > > > cmd line and terminated by '--'. In other words, it is designed assuming the DPDK library can dictate the application's command line. This is an incorrect assumption that should be eliminated. I don't think I'm the only one that has figured out that layering a service on top of DPDK requires creating a fake argc/argv array. The original plan of having rte_eal_init() parse the actual application command line organized as dictated by DPDK is not reasonable. > > > And 1 or 0 are not some > > > arbitrary values. They are used to put the index back to the beginning > > > of the new argv[] array. They're arbitrary values from the point of view of the calling application. If the caller is using getopt() for its own purposes, it has every reason to expect optind to have the same value after the call to rte_eal_init() that it had before, not some other value that the DPDK library happened to think was useful. > > > We shouldn't mix up DPDK's parameters and application's parameters. > > > And we should group them using '--'. Exactly! Yet we do confuse the two by using getopt() without considering that the application's parameters might not be in the argc/argv list that's passed to rte_eal_init(). > I'm considering updating optind to make it point to the option after > '--' before eal_parse_args() returns, and eal_parse_args() will return > 0 to avoid breaking the current applications which use the return value > of rte_eal_init() to update argc/argv. > > Any comments? Thanks! :-) Don't do it. Last time I looked, optind wasn't even mentioned in the documentation. Even if I thought it was reasonable to change it, I would still argue against using it as an undocumented return value, particularly when the documented return value works fine. -don provan dprovan@bivio.net ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] eal/bsd: reinitialize optind and optreset to 1 2015-10-14 17:54 ` Don Provan @ 2015-10-15 1:40 ` Tiwei Bie 0 siblings, 0 replies; 9+ messages in thread From: Tiwei Bie @ 2015-10-15 1:40 UTC (permalink / raw) To: Don Provan; +Cc: dev Hi Don! I'm truly sorry for my misunderstanding. :-( Thank you so much for your detailed comments! I will update my patch! Thanks again! Best wishes, Tiwei Bie On Wed, Oct 14, 2015 at 05:54:14PM +0000, Don Provan wrote: > > > On Wed, Oct 14, 2015 at 10:28:44AM +0800, Tiwei Bie wrote: > > > > It is designed to have DPDK's parameters specified in the front of the > > > > cmd line and terminated by '--'. > > In other words, it is designed assuming the DPDK library can dictate > the application's command line. This is an incorrect assumption > that should be eliminated. > > I don't think I'm the only one that has figured out that layering a > service on top of DPDK requires creating a fake argc/argv array. > The original plan of having rte_eal_init() parse the actual application > command line organized as dictated by DPDK is not reasonable. > > > > > And 1 or 0 are not some > > > > arbitrary values. They are used to put the index back to the beginning > > > > of the new argv[] array. > > They're arbitrary values from the point of view of the calling > application. If the caller is using getopt() for its own purposes, > it has every reason to expect optind to have the same value > after the call to rte_eal_init() that it had before, not some > other value that the DPDK library happened to think was > useful. > > > > > We shouldn't mix up DPDK's parameters and application's parameters. > > > > And we should group them using '--'. > > Exactly! Yet we do confuse the two by using getopt() without considering > that the application's parameters might not be in the argc/argv list that's > passed to rte_eal_init(). > > > I'm considering updating optind to make it point to the option after > > '--' before eal_parse_args() returns, and eal_parse_args() will return > > 0 to avoid breaking the current applications which use the return value > > of rte_eal_init() to update argc/argv. > > > > Any comments? Thanks! :-) > > Don't do it. Last time I looked, optind wasn't even mentioned in the > documentation. Even if I thought it was reasonable to change it, > I would still argue against using it as an undocumented return > value, particularly when the documented return value works fine. > > -don provan > dprovan@bivio.net ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-10-15 1:41 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-10-13 8:54 [dpdk-dev] [PATCH] Found a bug related to getopt() in eal/bsd module Tiwei Bie 2015-10-13 8:54 ` [dpdk-dev] [PATCH] eal/bsd: reinitialize optind and optreset to 1 Tiwei Bie 2015-10-13 14:58 ` Bruce Richardson 2015-10-13 17:14 ` Don Provan [not found] ` <20151014022843.GA26774@dell> 2015-10-14 9:31 ` Bruce Richardson 2015-10-14 10:19 ` Tiwei Bie 2015-10-14 11:28 ` Tiwei Bie 2015-10-14 17:54 ` Don Provan 2015-10-15 1:40 ` Tiwei Bie
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).