On Fri, Apr 13, 2018 at 08:19:16PM +0530, Shreyansh Jain wrote:
> On Friday 13 April 2018 06:52 PM, Gaetan Rivet wrote:
> > This function can be used as a callback to
> > rte_kvargs_process.
> > 
> > This should reduce code duplication.
> > 
> > Signed-off-by: Gaetan Rivet <gaetan.ri...@6wind.com>
> > ---
> >   lib/Makefile                             |  1 +
> >   lib/librte_kvargs/rte_kvargs.c           | 10 ++++++++++
> >   lib/librte_kvargs/rte_kvargs.h           | 28 ++++++++++++++++++++++++++++
> >   lib/librte_kvargs/rte_kvargs_version.map |  7 +++++++
> >   4 files changed, 46 insertions(+)
> > 
> > diff --git a/lib/Makefile b/lib/Makefile
> > index 1b17526f7..4206485d3 100644
> > --- a/lib/Makefile
> > +++ b/lib/Makefile
> > @@ -5,6 +5,7 @@ include $(RTE_SDK)/mk/rte.vars.mk
> >   DIRS-y += librte_compat
> >   DIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += librte_kvargs
> > +DEPDIRS-librte_kvargs := librte_compat
> >   DIRS-$(CONFIG_RTE_LIBRTE_EAL) += librte_eal
> >   DEPDIRS-librte_eal := librte_kvargs
> >   DIRS-$(CONFIG_RTE_LIBRTE_PCI) += librte_pci
> > diff --git a/lib/librte_kvargs/rte_kvargs.c b/lib/librte_kvargs/rte_kvargs.c
> > index 0a1abf579..6ee04cbb9 100644
> > --- a/lib/librte_kvargs/rte_kvargs.c
> > +++ b/lib/librte_kvargs/rte_kvargs.c
> > @@ -180,3 +180,13 @@ rte_kvargs_parse(const char *args, const char * const 
> > valid_keys[])
> >     return kvlist;
> >   }
> > +
> > +__rte_experimental
> > +int
> > +rte_kvargs_strcmp(const char *key __rte_unused,
> > +             const char *value, void *opaque)
> > +{
> > +   const char *str = opaque;
> > +
> > +   return -strcmp(str, value);
> > +}
> > diff --git a/lib/librte_kvargs/rte_kvargs.h b/lib/librte_kvargs/rte_kvargs.h
> > index 51b8120b8..c07c6fea5 100644
> > --- a/lib/librte_kvargs/rte_kvargs.h
> > +++ b/lib/librte_kvargs/rte_kvargs.h
> > @@ -25,6 +25,8 @@
> >   extern "C" {
> >   #endif
> > +#include <rte_compat.h>
> > +
> >   /** Maximum number of key/value associations */
> >   #define RTE_KVARGS_MAX 32
> > @@ -121,6 +123,32 @@ int rte_kvargs_process(const struct rte_kvargs *kvlist,
> >   unsigned rte_kvargs_count(const struct rte_kvargs *kvlist,
> >     const char *key_match);
> > +/**
> > + * Generic kvarg handler for string comparison.
> > + *
> > + * This function can be used for a generic string comparison processing
> > + * on a list of kvargs.
> > + *
> > + * @param key
> > + *   kvarg pair key.
> > + *
> > + * @param value
> > + *   kvarg pair value.
> > + *
> > + * @param opaque
> > + *   Opaque pointer to a string.
> > + *
> > + * @return
> > + *   0 if the strings match.
> > + *   !0 otherwise or on error.
> > + *
> > + *   Unless strcmp, comparison ordering is not kept.
> > + *   In order for rte_kvargs_process to stop processing on match error,
> > + *   a negative value is returned even if strcmp had returned a positive 
> > one.
> 
> Is the above comment valid?
> 
> > +   return -strcmp(str, value);
> 
> In case a negative is returned (when key opaque < value), this function
> would return a positive. So, effectively you have only reversed the values.
> Is that the expectation?
> 
> In 21/22:
> 
> --->8--- rte_kvargs_process ---
>     for (i = 0; i < kvlist->count; i++) {
>         pair = &kvlist->pairs[i];
>         if (key_match == NULL || strcmp(pair->key, key_match) == 0) {
>             if ((*handler)(pair->key, pair->value, opaque_arg) < 0)
>                 return -1;
>         }
>     }
> --->8---
> 
> This would only cause the opaque < value case to continue ahead but not the
> reverse.
> 

Ah! yes, you're right of course.
This is not the expectation, I will fix this.

Thanks,

-- 
Gaëtan Rivet
6WIND

Reply via email to