diff mbox

[dpdk-dev,v5,02/21] rte_string_fns.h: fix gcc8.1 sign conv warning in lstrcpy

Message ID 152656494207.46638.7698825480823239153.stgit@localhost.localdomain (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK

Commit Message

Andy Green May 17, 2018, 1:49 p.m. UTC 
  In file included from ./dpdk/dpdk.c:88:
/projects/lagopus/src/dpdk/build/include/rte_string_fns.h: In
function 'rte_strlcpy':
/projects/lagopus/src/dpdk/build/include/rte_string_fns.h:58:9:
warning: conversion to 'size_t' {aka 'long unsigned int'} from
'int' may change the sign of the result [-Wsign-conversion]
  return snprintf(dst, size, "%s", src);
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
---
 lib/librte_eal/common/include/rte_string_fns.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Bruce Richardson May 17, 2018, 1:56 p.m. UTC | #1 
On Thu, May 17, 2018 at 09:49:02PM +0800, Andy Green wrote:
> In file included from ./dpdk/dpdk.c:88:
> /projects/lagopus/src/dpdk/build/include/rte_string_fns.h: In
> function 'rte_strlcpy':
> /projects/lagopus/src/dpdk/build/include/rte_string_fns.h:58:9:
> warning: conversion to 'size_t' {aka 'long unsigned int'} from
> 'int' may change the sign of the result [-Wsign-conversion]
>   return snprintf(dst, size, "%s", src);
>          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ---
>  lib/librte_eal/common/include/rte_string_fns.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Stephen Hemminger May 17, 2018, 2:40 p.m. UTC | #2 
On Thu, 17 May 2018 21:49:02 +0800
Andy Green <andy@warmcat.com> wrote:

> In file included from ./dpdk/dpdk.c:88:
> /projects/lagopus/src/dpdk/build/include/rte_string_fns.h: In
> function 'rte_strlcpy':
> /projects/lagopus/src/dpdk/build/include/rte_string_fns.h:58:9:
> warning: conversion to 'size_t' {aka 'long unsigned int'} from
> 'int' may change the sign of the result [-Wsign-conversion]
>   return snprintf(dst, size, "%s", src);
>          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ---
>  lib/librte_eal/common/include/rte_string_fns.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/common/include/rte_string_fns.h b/lib/librte_eal/common/include/rte_string_fns.h
> index fcbb42e00..97597a148 100644
> --- a/lib/librte_eal/common/include/rte_string_fns.h
> +++ b/lib/librte_eal/common/include/rte_string_fns.h
> @@ -55,7 +55,7 @@ rte_strsplit(char *string, int stringlen,
>  static inline size_t
>  rte_strlcpy(char *dst, const char *src, size_t size)
>  {
> -	return snprintf(dst, size, "%s", src);
> +	return (size_t)snprintf(dst, size, "%s", src);
>  }
>  
>  /* pull in a strlcpy function */
> 

I still like the BSD function better because it guarantees all data
in the buffer is zero'd snprintf does not.
Bruce Richardson May 17, 2018, 3:28 p.m. UTC | #3 
On Thu, May 17, 2018 at 07:40:16AM -0700, Stephen Hemminger wrote:
> On Thu, 17 May 2018 21:49:02 +0800
> Andy Green <andy@warmcat.com> wrote:
> 
> > In file included from ./dpdk/dpdk.c:88:
> > /projects/lagopus/src/dpdk/build/include/rte_string_fns.h: In
> > function 'rte_strlcpy':
> > /projects/lagopus/src/dpdk/build/include/rte_string_fns.h:58:9:
> > warning: conversion to 'size_t' {aka 'long unsigned int'} from
> > 'int' may change the sign of the result [-Wsign-conversion]
> >   return snprintf(dst, size, "%s", src);
> >          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > ---
> >  lib/librte_eal/common/include/rte_string_fns.h |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/librte_eal/common/include/rte_string_fns.h b/lib/librte_eal/common/include/rte_string_fns.h
> > index fcbb42e00..97597a148 100644
> > --- a/lib/librte_eal/common/include/rte_string_fns.h
> > +++ b/lib/librte_eal/common/include/rte_string_fns.h
> > @@ -55,7 +55,7 @@ rte_strsplit(char *string, int stringlen,
> >  static inline size_t
> >  rte_strlcpy(char *dst, const char *src, size_t size)
> >  {
> > -	return snprintf(dst, size, "%s", src);
> > +	return (size_t)snprintf(dst, size, "%s", src);
> >  }
> >  
> >  /* pull in a strlcpy function */
> > 
> 
> I still like the BSD function better because it guarantees all data
> in the buffer is zero'd snprintf does not.

Right. But that is really a separate change from just fixing compiler
errors, which is why I think it's best kept out of this set.

As for zero-ing the rest of the buffer, reading the man page for strlcpy on
my fedora system, I find no mention of that behaviour. This implies to me
that the only guarantee from strlcpy is that there is one zero byte at the
end, and that the zeroing the rest of the array is not to be relied up.
Therefore, I see little value in having it in the fallback implementation
unless we are going to guarantee that we always use that implementation -
something we can't really do on freebsd systems, for example.

/Bruce
diff mbox

Patch

diff --git a/lib/librte_eal/common/include/rte_string_fns.h b/lib/librte_eal/common/include/rte_string_fns.h
index fcbb42e00..97597a148 100644
--- a/lib/librte_eal/common/include/rte_string_fns.h
+++ b/lib/librte_eal/common/include/rte_string_fns.h
@@ -55,7 +55,7 @@  rte_strsplit(char *string, int stringlen,
 static inline size_t
 rte_strlcpy(char *dst, const char *src, size_t size)
 {
-	return snprintf(dst, size, "%s", src);
+	return (size_t)snprintf(dst, size, "%s", src);
 }
 
 /* pull in a strlcpy function */