All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nathan Chancellor <nathan@kernel.org>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Kees Cook <keescook@chromium.org>,
	Francis Laniel <laniel_francis@privacyrequired.com>,
	Petr Mladek <pmladek@suse.com>,
	linux-kernel@vger.kernel.org, Andy Shevchenko <andy@kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	kernel test robot <lkp@intel.com>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Nathan Chancellor <natechancellor@gmail.com>
Subject: Re: [PATCH v3 1/3] string: Make stpcpy() possible to use
Date: Wed, 26 Jan 2022 09:38:02 -0700	[thread overview]
Message-ID: <YfF46oYCaelKU5qU@dev-arch.archlinux-ax161> (raw)
In-Reply-To: <20220126141917.75399-1-andriy.shevchenko@linux.intel.com>

On Wed, Jan 26, 2022 at 04:19:15PM +0200, Andy Shevchenko wrote:
> It is a good rule to avoid submitting code without users.

While I agree with the sentiment in the general case, I don't think that
it applies in this case and this comment should be dropped. The message
of the commit this fixes and the comment right above the declaration
both make it pretty obvious why this interface was added with no in-tree
users and why the declaration was placed right above the definition.

> Currently the stpcpy() is unusable due to missed declaration.
> Any attempts to use it will bring something like:
> 
>   error: implicit declaration of function ‘stpcpy’ [-Werror=implicit-function-declaration]
> 
> Move declaration to the header and guard it as other string functions.
> 
> Fixes: 1e1b6d63d634 ("lib/string.c: implement stpcpy")
> Reported-by: kernel test robot <lkp@intel.com>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Regardless, the commit itself seems fine from a technical standpoint. I
won't comment on whether or not this interface should be opened up.

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> v3: new patch to fix reported issue
>  include/linux/string.h | 3 +++
>  lib/string.c           | 3 ++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/string.h b/include/linux/string.h
> index b6572aeca2f5..b1aeb3475396 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -31,6 +31,9 @@ size_t strlcpy(char *, const char *, size_t);
>  #ifndef __HAVE_ARCH_STRSCPY
>  ssize_t strscpy(char *, const char *, size_t);
>  #endif
> +#ifndef __HAVE_ARCH_STPCPY
> +char *stpcpy(char *__restrict__ dest, const char *__restrict__ src);
> +#endif
>  
>  /* Wraps calls to strscpy()/memset(), no arch specific code required */
>  ssize_t strscpy_pad(char *dest, const char *src, size_t count);
> diff --git a/lib/string.c b/lib/string.c
> index 485777c9da83..4ecb8ec1fdd1 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -233,6 +233,7 @@ ssize_t strscpy(char *dest, const char *src, size_t count)
>  EXPORT_SYMBOL(strscpy);
>  #endif
>  
> +#ifndef __HAVE_ARCH_STPCPY
>  /**
>   * stpcpy - copy a string from src to dest returning a pointer to the new end
>   *          of dest, including src's %NUL-terminator. May overrun dest.
> @@ -248,7 +249,6 @@ EXPORT_SYMBOL(strscpy);
>   * not recommended for usage. Instead, its definition is provided in case
>   * the compiler lowers other libcalls to stpcpy.
>   */
> -char *stpcpy(char *__restrict__ dest, const char *__restrict__ src);
>  char *stpcpy(char *__restrict__ dest, const char *__restrict__ src)
>  {
>  	while ((*dest++ = *src++) != '\0')
> @@ -256,6 +256,7 @@ char *stpcpy(char *__restrict__ dest, const char *__restrict__ src)
>  	return --dest;
>  }
>  EXPORT_SYMBOL(stpcpy);
> +#endif
>  
>  #ifndef __HAVE_ARCH_STRCAT
>  /**
> -- 
> 2.34.1
> 
> 

  parent reply	other threads:[~2022-01-26 16:38 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-26 14:19 [PATCH v3 1/3] string: Make stpcpy() possible to use Andy Shevchenko
2022-01-26 14:19 ` [PATCH v3 2/3] vsprintf: Fix potential unaligned access Andy Shevchenko
2022-01-26 14:19 ` [PATCH v3 3/3] vsprintf: Move space out of string literals in fourcc_string() Andy Shevchenko
2022-01-26 21:22   ` Kees Cook
2022-01-27 11:04     ` Sakari Ailus
2022-01-27 14:57     ` Andy Shevchenko
2022-01-26 16:38 ` Nathan Chancellor [this message]
2022-01-26 18:04   ` [PATCH v3 1/3] string: Make stpcpy() possible to use Andy Shevchenko
2022-01-26 17:49 ` Nick Desaulniers
2022-01-26 18:07   ` Andy Shevchenko
2022-01-26 18:12   ` Andy Shevchenko
2022-01-26 21:09     ` Kees Cook

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=YfF46oYCaelKU5qU@dev-arch.archlinux-ax161 \
    --to=nathan@kernel.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=andy@kernel.org \
    --cc=keescook@chromium.org \
    --cc=laniel_francis@privacyrequired.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=lkp@intel.com \
    --cc=natechancellor@gmail.com \
    --cc=ndesaulniers@google.com \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.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.