All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Nicholas Mc Guire <hofrat@osadl.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH RFC] x86, boot: add missing declaration of string functions
Date: Thu, 5 Jan 2017 09:02:09 +0100	[thread overview]
Message-ID: <20170105080208.GC2098@gmail.com> (raw)
In-Reply-To: <1482487296-5064-1-git-send-email-hofrat@osadl.org>


* Nicholas Mc Guire <hofrat@osadl.org> wrote:

> Add the missing declarations of basic string functions to string.h to allow
> a clean build.
> 
> Fixes: commit 5be865661516 ("String-handling functions for the new x86 setup code.")
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> ---
> 
> sparse issues a set of warnings about missing declarations:
> arch/x86/purgatory/../boot/string.c:18:5: warning: symbol 'memcmp' was not declared. Should it be static?
> arch/x86/purgatory/../boot/string.c:26:5: warning: symbol 'strcmp' was not declared. Should it be static?
> arch/x86/purgatory/../boot/string.c:42:5: warning: symbol 'strncmp' was not declared. Should it be static?
> arch/x86/purgatory/../boot/string.c:58:8: warning: symbol 'strnlen' was not declared. Should it be static?
> arch/x86/purgatory/../boot/string.c:69:14: warning: symbol 'atou' was not declared. Should it be static?
> arch/x86/purgatory/../boot/string.c:99:20: warning: symbol 'simple_strtoull' was not declared. Should it be static?
> arch/x86/purgatory/../boot/string.c:128:8: warning: symbol 'strlen' was not declared. Should it be static?
> arch/x86/purgatory/../boot/string.c:142:6: warning: symbol 'strstr' was not declared. Should it be static?
> 
> This patch has one checkpatch warning about the use of simple_strtoul which
> is obsolete. As this is an independent implementation it is not clear if
> the changes made in simple_strtoul -> _kstrtoull might also need to be
> applied here ?
> 
> Patch was compile tested with: x86_64_defconfig
> 
> Patch is against 4.9.0 (localversion-next is next-20161223)
> 
>  arch/x86/boot/string.c |  1 +
>  arch/x86/boot/string.h |  9 +++++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/arch/x86/boot/string.c b/arch/x86/boot/string.c
> index cc3bd58..9e240fc 100644
> --- a/arch/x86/boot/string.c
> +++ b/arch/x86/boot/string.c
> @@ -14,6 +14,7 @@
>  
>  #include <linux/types.h>
>  #include "ctype.h"
> +#include "string.h"
>  
>  int memcmp(const void *s1, const void *s2, size_t len)
>  {
> diff --git a/arch/x86/boot/string.h b/arch/x86/boot/string.h
> index 725e820..f6ee139 100644
> --- a/arch/x86/boot/string.h
> +++ b/arch/x86/boot/string.h
> @@ -18,4 +18,13 @@ int memcmp(const void *s1, const void *s2, size_t len);
>  #define memset(d,c,l) __builtin_memset(d,c,l)
>  #define memcmp	__builtin_memcmp
>  
> +int strcmp(const char *str1, const char *str2);
> +int strncmp(const char *cs, const char *ct, size_t count);
> +size_t strlen(const char *s);
> +char *strstr(const char *s1, const char *s2);
> +size_t strnlen(const char *s, size_t maxlen);
> +unsigned int atou(const char *s);
> +unsigned long long simple_strtoull(const char *cp,
> +				    char **endp, unsigned int base);

Looks good to me, but please also mark them 'extern' to highlight the API 
declarations like the rest of the kernel does - such as kernel.h which has
the kernel's simple_strtoull() declaration, etc.

It's not required syntactically, but it's a good stylistic principle to keep 
external APIs organized.

Thanks,

	Ingo

  reply	other threads:[~2017-01-05  8:02 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-23 10:01 [PATCH RFC] x86, boot: add missing declaration of string functions Nicholas Mc Guire
2017-01-05  8:02 ` Ingo Molnar [this message]
2017-01-07  9:37   ` Nicholas Mc Guire

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170105080208.GC2098@gmail.com \
    --to=mingo@kernel.org \
    --cc=hofrat@osadl.org \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.