Patchwork [U-Boot,3/3] Add stricmp() and strnicmp()

login
register
mail settings
Submitter Simon Glass
Date Nov. 3, 2012, 9:45 p.m.
Message ID <1351979121-3769-3-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/196957/
State Rejected, archived
Headers show

Comments

Simon Glass - Nov. 3, 2012, 9:45 p.m.
strnicmp() is present but disabled. Make it available and define stricmp()
also. There is a only a small performance penalty to having stricmp()
call strnicmp(), so do this instead of a standalone function, to save code
space.

BRANCH=none
Signed-off-by: Simon Glass <sjg@chromium.org>
---
 include/linux/string.h |    4 ++++
 lib/string.c           |   12 ++++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)
Wolfgang Denk - Nov. 4, 2012, 12:31 a.m.
Dear Simon Glass,

In message <1351979121-3769-3-git-send-email-sjg@chromium.org> you wrote:
> strnicmp() is present but disabled. Make it available and define stricmp()
> also. There is a only a small performance penalty to having stricmp()
> call strnicmp(), so do this instead of a standalone function, to save code
> space.
> 
> BRANCH=none

Please get rid of such entries in the commit messages!!!!

Consider all patches that contain such entries as NAKed.

> +int strnicmp(const char *s1, const char *s2, size_t len);
> +
> +int stricmp(const char *s1, const char *s2);

Who are the users of this?   I object against adding dead code.

If users will be added later, this patch should go into the series
adding the users.

Best regards,

Wolfgang Denk
Simon Glass - Nov. 4, 2012, 4:38 a.m.
Hi Wolfgang,

On Sat, Nov 3, 2012 at 5:31 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon Glass,
>
> In message <1351979121-3769-3-git-send-email-sjg@chromium.org> you wrote:
>> strnicmp() is present but disabled. Make it available and define stricmp()
>> also. There is a only a small performance penalty to having stricmp()
>> call strnicmp(), so do this instead of a standalone function, to save code
>> space.
>>
>> BRANCH=none
>
> Please get rid of such entries in the commit messages!!!!
>
> Consider all patches that contain such entries as NAKed.

Yes it is annoying - I submitted a patch to patman to remove this and
another gerrit one also. In the meantime I have to be careful!

>
>> +int strnicmp(const char *s1, const char *s2, size_t len);
>> +
>> +int stricmp(const char *s1, const char *s2);
>
> Who are the users of this?   I object against adding dead code.
>
> If users will be added later, this patch should go into the series
> adding the users.

OK, messaged received loud and clear. It does require a change of
process at my end - now I have to find relationships between commits
in different series going to different maintainers and try to tie them
together. Just one more thing to worry about.

But I understand your concern that, in fact, if there is no user
immediately forthcoming, then it will just sit there and no one will
notice if it is dead code.

BTW, is there any easy way to obtain build-coverage information for
U-Boot? In other words, can we easily find code that is not enabled by
any existing board? That might be an interesting investigation.

Regards,
Simon

>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
> Severe culture shock results when experts from another protocol suite
> [...] try to read OSI documents. The term "osified" is used to  refer
> to  such  documents. [...] Any relationship to the word "ossified" is
> purely intentional.                                - Marshall T. Rose
Albert ARIBAUD - Nov. 4, 2012, 10:47 p.m.
Hi Simon,

> OK, messaged received loud and clear. It does require a change of
> process at my end - now I have to find relationships between commits
> in different series going to different maintainers and try to tie them
> together. Just one more thing to worry about.

Consider a 'paradigm shift' here: instead of building series according
to intended custodians, one builds them according to functional
relationship. If a series needs one custodian more than the others,
he'll take it and ask for the others' ack. If there's a tie, this can be
resolved between custodians. If one foresees a tie, then one can
proactively suggest a resolution.

> But I understand your concern that, in fact, if there is no user
> immediately forthcoming, then it will just sit there and no one will
> notice if it is dead code.
> 
> BTW, is there any easy way to obtain build-coverage information for
> U-Boot? In other words, can we easily find code that is not enabled by
> any existing board? That might be an interesting investigation.

Seconded -- with the added note that we need coverage across all
architectures.

> Regards,
> Simon

Amicalement,
Simon Glass - Nov. 7, 2012, 10 p.m.
Hi,

On Sun, Nov 4, 2012 at 2:47 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Hi Simon,
>
>> OK, messaged received loud and clear. It does require a change of
>> process at my end - now I have to find relationships between commits
>> in different series going to different maintainers and try to tie them
>> together. Just one more thing to worry about.
>
> Consider a 'paradigm shift' here: instead of building series according
> to intended custodians, one builds them according to functional
> relationship. If a series needs one custodian more than the others,
> he'll take it and ask for the others' ack. If there's a tie, this can be
> resolved between custodians. If one foresees a tie, then one can
> proactively suggest a resolution.
>
>> But I understand your concern that, in fact, if there is no user
>> immediately forthcoming, then it will just sit there and no one will
>> notice if it is dead code.
>>
>> BTW, is there any easy way to obtain build-coverage information for
>> U-Boot? In other words, can we easily find code that is not enabled by
>> any existing board? That might be an interesting investigation.
>
> Seconded -- with the added note that we need coverage across all
> architectures.

This patch is needed by new exynos memory init code. I am copying
Hatim so that he is aware that it will need to be sent as part of his
memory init series, which I believe is coming soon.

We can leave this for now.

Regards,
Simon

>
>> Regards,
>> Simon
>
> Amicalement,
> --
> Albert.
Simon Glass - Nov. 22, 2012, 2:51 a.m.
Hi,

On Wed, Nov 7, 2012 at 2:00 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi,
>
> On Sun, Nov 4, 2012 at 2:47 PM, Albert ARIBAUD
> <albert.u.boot@aribaud.net> wrote:
>> Hi Simon,
>>
>>> OK, messaged received loud and clear. It does require a change of
>>> process at my end - now I have to find relationships between commits
>>> in different series going to different maintainers and try to tie them
>>> together. Just one more thing to worry about.
>>
>> Consider a 'paradigm shift' here: instead of building series according
>> to intended custodians, one builds them according to functional
>> relationship. If a series needs one custodian more than the others,
>> he'll take it and ask for the others' ack. If there's a tie, this can be
>> resolved between custodians. If one foresees a tie, then one can
>> proactively suggest a resolution.
>>
>>> But I understand your concern that, in fact, if there is no user
>>> immediately forthcoming, then it will just sit there and no one will
>>> notice if it is dead code.
>>>
>>> BTW, is there any easy way to obtain build-coverage information for
>>> U-Boot? In other words, can we easily find code that is not enabled by
>>> any existing board? That might be an interesting investigation.
>>
>> Seconded -- with the added note that we need coverage across all
>> architectures.
>
> This patch is needed by new exynos memory init code. I am copying
> Hatim so that he is aware that it will need to be sent as part of his
> memory init series, which I believe is coming soon.
>
> We can leave this for now.

As it happens I think I need this for the hashing code, so I will
include this patch when I resend the common/ series.

Regards,
Simon

>
> Regards,
> Simon
>
>>
>>> Regards,
>>> Simon
>>
>> Amicalement,
>> --
>> Albert.

Patch

diff --git a/include/linux/string.h b/include/linux/string.h
index 9a8cbc2..77fd1e9 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -20,6 +20,10 @@  extern __kernel_size_t strspn(const char *,const char *);
  */
 #include <asm/string.h>
 
+int strnicmp(const char *s1, const char *s2, size_t len);
+
+int stricmp(const char *s1, const char *s2);
+
 #ifndef __HAVE_ARCH_STRCPY
 extern char * strcpy(char *,const char *);
 #endif
diff --git a/lib/string.c b/lib/string.c
index c3ad055..f73df3f 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -21,7 +21,6 @@ 
 #include <malloc.h>
 
 
-#if 0 /* not used - was: #ifndef __HAVE_ARCH_STRNICMP */
 /**
  * strnicmp - Case insensitive, length-limited string comparison
  * @s1: One string
@@ -52,7 +51,16 @@  int strnicmp(const char *s1, const char *s2, size_t len)
 	}
 	return (int)c1 - (int)c2;
 }
-#endif
+
+/**
+ * stricmp - Case insensitive string comparison
+ * @s1: One string
+ * @s2: The other string
+ */
+int stricmp(const char *s1, const char *s2)
+{
+	return strnicmp(s1, s2, -1U);
+}
 
 char * ___strtok;