[dpdk-dev] [PATCH] cmdline: fix type format from unsigned to size_t for buffer size

[dpdk-dev] [PATCH] cmdline: fix type format from unsigned to size_t for buffer size

[Daniel Mrzyglod]

Function match_inst is used to take buffor using sizeof() which is size_t type.
This modification also involved changing '%u' to '%zu' in printf function.

Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
---
 lib/librte_cmdline/cmdline_parse.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_cmdline/cmdline_parse.c b/lib/librte_cmdline/cmdline_parse.c
index dfc885c..0821791 100644
--- a/lib/librte_cmdline/cmdline_parse.c
+++ b/lib/librte_cmdline/cmdline_parse.c
@@ -138,7 +138,7 @@ nb_common_chars(const char * s1, const char * s2)
  */
 static int
 match_inst(cmdline_parse_inst_t *inst, const char *buf,
-	   unsigned int nb_match_token, void *resbuf, unsigned resbuf_size)
+	   unsigned int nb_match_token, void *resbuf, size_t resbuf_size)
 {
 	unsigned int token_num=0;
 	cmdline_parse_token_hdr_t * token_p;
@@ -169,7 +169,7 @@ match_inst(cmdline_parse_inst_t *inst, const char *buf,
 
 			if (token_hdr.offset > resbuf_size) {
 				printf("Parse error(%s:%d): Token offset(%u) "
-					"exceeds maximum size(%u)\n",
+					"exceeds maximum size(%zu)\n",
 					__FILE__, __LINE__,
 					token_hdr.offset, resbuf_size);
 				return -ENOBUFS;
-- 
2.1.0

Re: [dpdk-dev] [PATCH] cmdline: fix type format from unsigned to size_t for buffer size

[Olivier MATZ]

Hi Daniel,

On 02/20/2015 05:18 PM, Daniel Mrzyglod wrote:
> Function match_inst is used to take buffor using sizeof() which is size_t type.
> This modification also involved changing '%u' to '%zu' in printf function.
>
> Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
> ---
>   lib/librte_cmdline/cmdline_parse.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_cmdline/cmdline_parse.c b/lib/librte_cmdline/cmdline_parse.c
> index dfc885c..0821791 100644
> --- a/lib/librte_cmdline/cmdline_parse.c
> +++ b/lib/librte_cmdline/cmdline_parse.c
> @@ -138,7 +138,7 @@ nb_common_chars(const char * s1, const char * s2)
>    */
>   static int
>   match_inst(cmdline_parse_inst_t *inst, const char *buf,
> -	   unsigned int nb_match_token, void *resbuf, unsigned resbuf_size)
> +	   unsigned int nb_match_token, void *resbuf, size_t resbuf_size)
>   {
>   	unsigned int token_num=0;
>   	cmdline_parse_token_hdr_t * token_p;
> @@ -169,7 +169,7 @@ match_inst(cmdline_parse_inst_t *inst, const char *buf,
>
>   			if (token_hdr.offset > resbuf_size) {
>   				printf("Parse error(%s:%d): Token offset(%u) "
> -					"exceeds maximum size(%u)\n",
> +					"exceeds maximum size(%zu)\n",
>   					__FILE__, __LINE__,
>   					token_hdr.offset, resbuf_size);
>   				return -ENOBUFS;
>


Did you see a specific issue with the current code? (maybe a compilation
issue or a klocwork issue?)

I think this patch is ok, but there are many places where this kind
of fixes should be applied in cmdline (cmdline_parse_*(),
cmdline_get_help_*(), etc.). Is there a motivation for changing it
only there?

Regards,
Olivier

Re: [dpdk-dev] [PATCH] cmdline: fix type format from unsigned to size_t for buffer size

[Thomas Monjalon]

Daniel,
Without answer to the question from Olivier, this patch is going to be ignored.

2015-02-24 12:03, Olivier MATZ:
> Hi Daniel,
> 
> On 02/20/2015 05:18 PM, Daniel Mrzyglod wrote:
> > Function match_inst is used to take buffor using sizeof() which is size_t type.
> > This modification also involved changing '%u' to '%zu' in printf function.
> >
> > Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
> > ---
> >   lib/librte_cmdline/cmdline_parse.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/librte_cmdline/cmdline_parse.c b/lib/librte_cmdline/cmdline_parse.c
> > index dfc885c..0821791 100644
> > --- a/lib/librte_cmdline/cmdline_parse.c
> > +++ b/lib/librte_cmdline/cmdline_parse.c
> > @@ -138,7 +138,7 @@ nb_common_chars(const char * s1, const char * s2)
> >    */
> >   static int
> >   match_inst(cmdline_parse_inst_t *inst, const char *buf,
> > -	   unsigned int nb_match_token, void *resbuf, unsigned resbuf_size)
> > +	   unsigned int nb_match_token, void *resbuf, size_t resbuf_size)
> >   {
> >   	unsigned int token_num=0;
> >   	cmdline_parse_token_hdr_t * token_p;
> > @@ -169,7 +169,7 @@ match_inst(cmdline_parse_inst_t *inst, const char *buf,
> >
> >   			if (token_hdr.offset > resbuf_size) {
> >   				printf("Parse error(%s:%d): Token offset(%u) "
> > -					"exceeds maximum size(%u)\n",
> > +					"exceeds maximum size(%zu)\n",
> >   					__FILE__, __LINE__,
> >   					token_hdr.offset, resbuf_size);
> >   				return -ENOBUFS;
> >
> 
> 
> Did you see a specific issue with the current code? (maybe a compilation
> issue or a klocwork issue?)
> 
> I think this patch is ok, but there are many places where this kind
> of fixes should be applied in cmdline (cmdline_parse_*(),
> cmdline_get_help_*(), etc.). Is there a motivation for changing it
> only there?
> 
> Regards,
> Olivier
> 

Re: [dpdk-dev] [PATCH] cmdline: fix type format from unsigned to size_t for buffer size

[Jastrzebski, MichalX K]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Monday, March 23, 2015 12:47 PM
> To: Mrzyglod, DanielX T
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] cmdline: fix type format from unsigned to
> size_t for buffer size
> 
> Daniel,
> Without answer to the question from Olivier, this patch is going to be
> ignored.
> 
> 2015-02-24 12:03, Olivier MATZ:
> > Hi Daniel,
> >
> > On 02/20/2015 05:18 PM, Daniel Mrzyglod wrote:
> > > Function match_inst is used to take buffor using sizeof() which is size_t
> type.
> > > This modification also involved changing '%u' to '%zu' in printf function.
> > >
> > > Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
> > > ---
> > >   lib/librte_cmdline/cmdline_parse.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/lib/librte_cmdline/cmdline_parse.c
> b/lib/librte_cmdline/cmdline_parse.c
> > > index dfc885c..0821791 100644
> > > --- a/lib/librte_cmdline/cmdline_parse.c
> > > +++ b/lib/librte_cmdline/cmdline_parse.c
> > > @@ -138,7 +138,7 @@ nb_common_chars(const char * s1, const char *
> s2)
> > >    */
> > >   static int
> > >   match_inst(cmdline_parse_inst_t *inst, const char *buf,
> > > -	   unsigned int nb_match_token, void *resbuf, unsigned resbuf_size)
> > > +	   unsigned int nb_match_token, void *resbuf, size_t resbuf_size)
> > >   {
> > >   	unsigned int token_num=0;
> > >   	cmdline_parse_token_hdr_t * token_p;
> > > @@ -169,7 +169,7 @@ match_inst(cmdline_parse_inst_t *inst, const
> char *buf,
> > >
> > >   			if (token_hdr.offset > resbuf_size) {
> > >   				printf("Parse error(%s:%d): Token offset(%u)
> "
> > > -					"exceeds maximum size(%u)\n",
> > > +					"exceeds maximum size(%zu)\n",
> > >   					__FILE__, __LINE__,
> > >   					token_hdr.offset, resbuf_size);
> > >   				return -ENOBUFS;
> > >
> >
> >
> > Did you see a specific issue with the current code? (maybe a compilation
> > issue or a klocwork issue?)
Hi Olivier, Thomas
Yes, this is an issue reported by the static analysis tool.
> >
> > I think this patch is ok, but there are many places where this kind
> > of fixes should be applied in cmdline (cmdline_parse_*(),
> > cmdline_get_help_*(), etc.). Is there a motivation for changing it
> > only there?
The tool we use didn't reported other places, 
that is why Daniel send only this one change.
> >
> > Regards,
> > Olivier
> >
> 

Re: [dpdk-dev] [PATCH] cmdline: fix type format from unsigned to size_t for buffer size

[Olivier MATZ]

Hi,

On 03/24/2015 11:48 AM, Jastrzebski, MichalX K wrote:
>>> On 02/20/2015 05:18 PM, Daniel Mrzyglod wrote:
>>>> Function match_inst is used to take buffor using sizeof() which is size_t
>> type.
>>>> This modification also involved changing '%u' to '%zu' in printf function.
>>>>
>>>> Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
>>>>
>>>> [...]
>>>
>>> Did you see a specific issue with the current code? (maybe a compilation
>>> issue or a klocwork issue?)
> Hi Olivier, Thomas
> Yes, this is an issue reported by the static analysis tool.
>>>
>>> I think this patch is ok, but there are many places where this kind
>>> of fixes should be applied in cmdline (cmdline_parse_*(),
>>> cmdline_get_help_*(), etc.). Is there a motivation for changing it
>>> only there?
> The tool we use didn't reported other places,
> that is why Daniel send only this one change.

I think it would be great to look at other places to check if the same
change is required.

Regards,
Olivier