From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet Subject: Re: [PATCH v6 12/22] kvargs: add generic string matching callback Date: Fri, 13 Apr 2018 17:06:42 +0200 Message-ID: <20180413150642.rcdsx5ovyu4o5zff@bidouze.vm.6wind.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Cc: dev@dpdk.org To: Shreyansh Jain Return-path: Received: from mail-wr0-f193.google.com (mail-wr0-f193.google.com [209.85.128.193]) by dpdk.org (Postfix) with ESMTP id 2CCA11B6C3 for ; Fri, 13 Apr 2018 17:06:57 +0200 (CEST) Received: by mail-wr0-f193.google.com with SMTP id l49so8965156wrl.4 for ; Fri, 13 Apr 2018 08:06:57 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > > --- > > 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 > > + > > /** 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