Re: [dpdk-dev] [PATCH 1.6.1] don't inline rte_string_fns

[Thomas Monjalon <thomas.monjalon@6wind.com>]

Hi Stephen,
This patch is still pending.
Please send a v2.

2014-03-20 17:30, Thomas Monjalon:
> Hi,
> 
> I have some minor comments below.
> 
> 27/02/2014 09:18, Stephen Hemminger :
> > The function rte_snprintf() can never be inlined by Gcc.
> > 
> > If compiled with -Winline it generates an error:
> >  function ‘rte_snprintf’ can never be inlined because it uses variable
> > 
> > argument lists [-Werror=inline]
> > 
> > Therefore since both rte_snprintf and rte_strsplit are not performance
> > sensitive just move them to being real functions.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > 
> > --- /dev/null
> > +++ b/lib/librte_eal/common/eal_common_string_fns.c
> > @@ -0,0 +1,95 @@
> > +/*-
> > + *   BSD LICENSE
> > + *
> > + *   Copyright(c) 2010-2013 Intel Corporation. All rights reserved.
> 
> It is now 2014 in version 1.6.0.
> 
> [...]
> 
> > +#include <stdio.h>
> > +#include <stdarg.h>
> > +#include <stddef.h>
> > +#include <errno.h>
> 
> I think stddef.h is not needed.
> 
> [...]
> 
> > --- a/lib/librte_eal/common/include/rte_string_fns.h
> > +++ b/lib/librte_eal/common/include/rte_string_fns.h
> > @@ -47,7 +47,6 @@ extern "C" {
> > 
> >  #include <stdio.h>
> >  #include <stdarg.h>
> >  #include <stddef.h>
> > 
> > -#include <errno.h>
> 
> stdarg and stddef are not needed.
> 
> [...]
> 
> > +int
> > +rte_snprintf(char *buffer, int buflen, const char *format, ...);
> 
> One blank line should be sufficient.
> 
> > +int
> > 
> >  rte_strsplit(char *string, int stringlen,
> 
> Thank you