> -----Original Message----- > From: Yang, Ziye > Sent: Wednesday, May 11, 2016 2:39 PM > To: Richardson, Bruce <bruce.richardson at intel.com> > Cc: dev at dpdk.org > Subject: RE: [dpdk-dev] [PATCH] librte_eal: fix wrong args operation in > eal_parse_args > > Hi Bruce, > > Could you tell me the documentation file name? Then I can conduct the > following documentation relate d patch.
The behavior is already documented in the doxygen API definitions for rte_eal_init(). * @return$ * - On success, the number of parsed arguments, which is greater or$ * equal to zero. After the call to rte_eal_init(),$ * all arguments argv[x] with x < ret may be modified and should$ * not be accessed by the application.$ * - On failure, a negative error value.$ However, maybe you want to call it out in a different way to make it clearer for those using the functions. Other than that, perhaps look in the programmers guide document to see if it needs a mention there - or a reference to the API doc. /Bruce PS: When replying, please respond below existing emails, rather than top-posting. Thanks. > > -----Original Message----- > From: Richardson, Bruce > Sent: Wednesday, May 11, 2016 8:21 PM > To: Yang, Ziye <ziye.yang at intel.com> > Cc: dev at dpdk.org > Subject: RE: [dpdk-dev] [PATCH] librte_eal: fix wrong args operation in > eal_parse_args > > > > > -----Original Message----- > > From: Yang, Ziye > > Sent: Wednesday, May 11, 2016 12:51 PM > > To: Richardson, Bruce <bruce.richardson at intel.com> > > Cc: dev at dpdk.org > > Subject: RE: [dpdk-dev] [PATCH] librte_eal: fix wrong args operation > > in eal_parse_args > > > > Hi Bruce, > > > > Could it be fixed later? If not, it should be documented. I faced > > this issued today, and found that dpdk changed my last arg. In my > > mind, dpdk should not change the argv[last], which will confuse the > users. > > > > We can certainly consider changing it, but I am concerned about stability > of existing apps. I think it needs some discussion and consensus on-list > before a change like this is made. > > For right now, definitely the behavior should be documented. Would you > consider submitting a documentation update patch for this issue instead of > this code change? > > Thanks, > /Bruce > > > Best Regards, > > Ziye Yang > > > > -----Original Message----- > > From: Richardson, Bruce > > Sent: Wednesday, May 11, 2016 7:25 PM > > To: Yang, Ziye <ziye.yang at intel.com> > > Cc: dev at dpdk.org > > Subject: Re: [dpdk-dev] [PATCH] librte_eal: fix wrong args operation > > in eal_parse_args > > > > On Wed, May 11, 2016 at 01:28:21PM +0800, Ziye Yang wrote: > > > This patch is used to fix wrong operation on user input args. > > > eal_parse_args function should not operate the args passed by the > > > user. If the element in argv is generated by malloc function, > > > changing it will cause memory issues when free the args. > > > > > > Signed-off-by: Ziye Yang <ziye.yang at intel.com> > > > --- > > > lib/librte_eal/bsdapp/eal/eal.c | 2 -- > > > lib/librte_eal/linuxapp/eal/eal.c | 2 -- > > > 2 files changed, 4 deletions(-) > > > > > > diff --git a/lib/librte_eal/bsdapp/eal/eal.c > > > b/lib/librte_eal/bsdapp/eal/eal.c index 06bfd4e..0eef92d 100644 > > > --- a/lib/librte_eal/bsdapp/eal/eal.c > > > +++ b/lib/librte_eal/bsdapp/eal/eal.c > > > @@ -420,8 +420,6 @@ eal_parse_args(int argc, char **argv) > > > goto out; > > > } > > > > > > - if (optind >= 0) > > > - argv[optind-1] = prgname; > > > ret = optind-1; > > > > > > out: > > > diff --git a/lib/librte_eal/linuxapp/eal/eal.c > > > b/lib/librte_eal/linuxapp/eal/eal.c > > > index 8aafd51..ba9d1ac 100644 > > > --- a/lib/librte_eal/linuxapp/eal/eal.c > > > +++ b/lib/librte_eal/linuxapp/eal/eal.c > > > @@ -658,8 +658,6 @@ eal_parse_args(int argc, char **argv) > > > goto out; > > > } > > > > > > - if (optind >= 0) > > > - argv[optind-1] = prgname; > > > ret = optind-1; > > > > > > out: > > This is a behaviour change in DPDK. The behaviour has always been that > > after calling eal init, you can update your argv/argc values by the > > number of args parsed and then parse your app args as normal, since > > argv[0] will still point to your program name. While I agree that > > having the current behaviour may cause some problems, changing this > > behaviour may break applications that have been written to use the > existing behaviour. > > > > Since it is only the last EAL parameter arg that is modified, I think > > it would be acceptable to have the behaviour well documented and then > > expect any app to store a second copy of the pointer to be modified if > > it is needed for a subsequent free call, for example. > > > > /Bruce