DPDK patches and discussions
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: Ziye Yang <ziye.yang@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] librte_eal: fix wrong args operation in eal_parse_args
Date: Wed, 11 May 2016 12:24:51 +0100	[thread overview]
Message-ID: <20160511112451.GA18868@bricha3-MOBL3> (raw)
In-Reply-To: <1462944501-15852-1-git-send-email-ziye.yang@intel.com>

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@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

  reply	other threads:[~2016-05-11 11:29 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-11  5:28 Ziye Yang
2016-05-11 11:24 ` Bruce Richardson [this message]
2016-05-11 11:51   ` Yang, Ziye
2016-05-11 12:21     ` Richardson, Bruce
2016-05-11 13:39       ` Yang, Ziye
2016-05-11 14:07         ` Richardson, Bruce

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=20160511112451.GA18868@bricha3-MOBL3 \
    --to=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=ziye.yang@intel.com \
    /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).