[dpdk-dev,v6,12/22] kvargs: add generic string matching callback

Message ID d6011a126f4474f4d3a6f3555eea5b2ae9b342b6.1523625525.git.gaetan.rivet@6wind.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply patch file failure

Commit Message

Gaëtan Rivet April 13, 2018, 1:22 p.m. UTC
  This function can be used as a callback to
rte_kvargs_process.

This should reduce code duplication.

Signed-off-by: Gaetan Rivet <gaetan.rivet@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(+)
  

Comments

Shreyansh Jain April 13, 2018, 2:49 p.m. UTC | #1
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.rivet@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.

> + */
> +__rte_experimental
> +int rte_kvargs_strcmp(const char *key, const char *value, void *opaque);
> +
>   #ifdef __cplusplus
>   }
>   #endif
> diff --git a/lib/librte_kvargs/rte_kvargs_version.map b/lib/librte_kvargs/rte_kvargs_version.map
> index 2030ec46c..e2f663a88 100644
> --- a/lib/librte_kvargs/rte_kvargs_version.map
> +++ b/lib/librte_kvargs/rte_kvargs_version.map
> @@ -8,3 +8,10 @@ DPDK_2.0 {
>   
>   	local: *;
>   };
> +
> +EXPERIMENTAL {
> +	global:
> +
> +	rte_kvargs_strcmp;
> +
> +} DPDK_2.0;
>
  
Gaëtan Rivet April 13, 2018, 3:06 p.m. UTC | #2
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.rivet@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,
  

Patch

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.
+ */
+__rte_experimental
+int rte_kvargs_strcmp(const char *key, const char *value, void *opaque);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_kvargs/rte_kvargs_version.map b/lib/librte_kvargs/rte_kvargs_version.map
index 2030ec46c..e2f663a88 100644
--- a/lib/librte_kvargs/rte_kvargs_version.map
+++ b/lib/librte_kvargs/rte_kvargs_version.map
@@ -8,3 +8,10 @@  DPDK_2.0 {
 
 	local: *;
 };
+
+EXPERIMENTAL {
+	global:
+
+	rte_kvargs_strcmp;
+
+} DPDK_2.0;