From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753372AbcAGWFQ (ORCPT ); Thu, 7 Jan 2016 17:05:16 -0500 Received: from mail-wm0-f54.google.com ([74.125.82.54]:37598 "EHLO mail-wm0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752015AbcAGWFN (ORCPT ); Thu, 7 Jan 2016 17:05:13 -0500 From: Rasmus Villemoes To: Andy Shevchenko Cc: Tejun Heo , Linus Walleij , Dmitry Eremin-Solenikov , linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, "David S. Miller" , David Airlie , Andrew Morton Subject: Re: [PATCH v1 1/8] lib/string: introduce match_string() helper Organization: D03 References: <1452168368-75630-1-git-send-email-andriy.shevchenko@linux.intel.com> X-Hashcash: 1:20:160107:davem@davemloft.net::a/C4zKkBNfxJNrlu:00000000000000000000000000000000000000000000Rv X-Hashcash: 1:20:160107:linus.walleij@linaro.org::IXD4DOY4+8XgqgyR:00000000000000000000000000000000000000ezw X-Hashcash: 1:20:160107:andriy.shevchenko@linux.intel.com::TZ1FSohoif9lK4rA:00000000000000000000000000000Cb9 X-Hashcash: 1:20:160107:linux-pm@vger.kernel.org::3WUcsMIUdWrACvab:00000000000000000000000000000000000000sSl X-Hashcash: 1:20:160107:akpm@linux-foundation.org::InIdVEi9G8oWDzt2:00000000000000000000000000000000000018dN X-Hashcash: 1:20:160107:tj@kernel.org::HUA54rVn4w5nKKGn:00001iOw X-Hashcash: 1:20:160107:dbaryshkov@gmail.com::TycDTbRRFGIgbU/+:000000000000000000000000000000000000000003gem X-Hashcash: 1:20:160107:linux-kernel@vger.kernel.org::gwBoBqVUY+pctNZV:0000000000000000000000000000000002t98 X-Hashcash: 1:20:160107:airlied@linux.ie::ZgYocUR8cLE0GHay:0GH2b Date: Thu, 07 Jan 2016 23:05:09 +0100 In-Reply-To: <1452168368-75630-1-git-send-email-andriy.shevchenko@linux.intel.com> (Andy Shevchenko's message of "Thu, 7 Jan 2016 14:06:01 +0200") Message-ID: <874mepnizu.fsf@rasmusvillemoes.dk> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jan 07 2016, Andy Shevchenko wrote: > From time to time we have to match a string in an array. Make a simple helper > for that purpose. > > Signed-off-by: Andy Shevchenko > --- > include/linux/string.h | 2 ++ > lib/string.c | 26 ++++++++++++++++++++++++++ > 2 files changed, 28 insertions(+) > > diff --git a/include/linux/string.h b/include/linux/string.h > index b0a732b..37062fb 100644 > --- a/include/linux/string.h > +++ b/include/linux/string.h > @@ -131,6 +131,8 @@ extern void argv_free(char **argv); > extern bool sysfs_streq(const char *s1, const char *s2); > extern int strtobool(const char *s, bool *res); > > +int match_string(const char * const *array, size_t len, const char *string); > + > #ifdef CONFIG_BINARY_PRINTF > int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args); > int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf); > diff --git a/lib/string.c b/lib/string.c > index 0323c0d..dd02270 100644 > --- a/lib/string.c > +++ b/lib/string.c > @@ -631,6 +631,32 @@ bool sysfs_streq(const char *s1, const char *s2) > EXPORT_SYMBOL(sysfs_streq); > > /** > + * match_string - matches given string in an array > + * @array: array of strings > + * @len: number of strings in the array or 0 for NULL terminated arrays > + * @string: string to match with > + * > + * Return: > + * index of a @string in the @array if matches, or %-ENODATA otherwise. > + */ > +int match_string(const char * const *array, size_t len, const char *string) > +{ > + int index = 0; > + const char *item; > + > + do { > + item = array[index]; > + if (!item) > + break; > + if (!strcmp(item, string)) > + return index; > + } while (++index < len || !len); > + > + return -ENODATA; > +} > +EXPORT_SYMBOL(match_string); > + I'd suggest making it -1 (which, since len is a size_t, is effectively infinity) having the meaning "the array is terminated by a NULL entry". match_string(..., ARRAY_SIZE(my_array), ...) will break if the array happens to be empty, which could e.g. happen in a case like const char *my_array[] = { #ifdef CONFIG_THIS "this", #endif #ifdef CONFIG_THAT "that", #endif }; I also think the condition/loop above is unreadable. for (index = 0; index < len; index++) { ... } is much clearer. Why -ENODATA and not just -1? It is rather unlikely that anyone would pass on that particular -Exxx value. Not a biggie, just curious. Would there be more potential users if we had a flag argument allowing case-insensitive matching? Would there be more potential users if a flag allowed to ask whether the given string is a _prefix_ of one of the strings in the array, or vice versa? Something like #define MATCH_STRING_CASE 0x01 #define MATCH_STRING_PREFIX_OF_ARRAY_ELEM 0x02 /* yeah, that name sucks */ #define MATCH_ARRAY_ELEM_PREFIX_OF_STRING 0x04 /* this too */ int match_string(const char * const *array, size_t len, const char *string, unsigned flags) { #define MATCH_PREFIX (MATCH_... | MATCH_...) int index; const char *item; int (*match_func)(const char *, const char *) = flags & MATCH_STRING_CASE ? strcasecmp : strcmp; int (*prefix_func)(const char *, const char *, size_t) = flags & MATCH_STRING_CASE ? strncasecmp : strncmp; for (index = 0; index < len; ++index) { item = array[index]; if (!item) break; if (flags & MATCH_PREFIX) { size_t len = strlen(flags & MATCH_STRING_PREFIX_OF_ARRAY_ELEM ? string : item); if (!prefix_func(item, string, len)) return index; } else if (!match_func(item, string)) { return index; } } return -1; } (Ok, it's not that pretty; maybe it'd be better to use switch(flags&MATCH_PREFIX) {}. Or maybe just the case-insensitive part is worth keeping; in that case the above isn't that bad.) Rasmus