public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
From: david laight <david.laight@runbox.com>
To: Thorsten Blum <thorsten.blum@linux.dev>
Cc: Chris Mason <clm@fb.com>, David Sterba <dsterba@suse.com>,
	linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] btrfs: Replace memcpy + NUL termination in _btrfs_printk
Date: Sun, 30 Nov 2025 11:06:40 +0000	[thread overview]
Message-ID: <20251130110640.21eadec5@pumpkin> (raw)
In-Reply-To: <20251130005518.82065-1-thorsten.blum@linux.dev>

On Sun, 30 Nov 2025 01:55:17 +0100
Thorsten Blum <thorsten.blum@linux.dev> wrote:

> Use strscpy() to copy the NUL-terminated source string 'fmt' to the
> destination buffer 'lvl' instead of using memcpy() followed by a manual
> NUL termination.  No functional changes.

Why?
The code has just got the length of part of the format string, it wants
a copy of it with a '\0' terminator.
So memcpy() is correct and strscpy() just expensive.
The code is actually strange (and strangely written), but 'size' is always 2.

One might question why btrfs has to invent its own 'printk' scheme...

But PRINTK_MAX_SINGLE_HEADER_LEN is only used here - so take out the 'MAX_'
and use it inside printk_skip_level'.
Then here use the constant PRINTK_SINGLE_HEADER_LEN instead of 'size'.
Since lvl[] is initialised it will all be zero, so after the
mempy(lvl, fmt, 2) (which will be inlined) there is no need to the
lvl[2] = 0 at all.

	David

> 
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  fs/btrfs/messages.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/messages.c b/fs/btrfs/messages.c
> index a0cf8effe008..083e228e6d6c 100644
> --- a/fs/btrfs/messages.c
> +++ b/fs/btrfs/messages.c
> @@ -1,5 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  
> +#include <linux/string.h>
>  #include "fs.h"
>  #include "messages.h"
>  #include "discard.h"
> @@ -229,8 +230,7 @@ void __cold _btrfs_printk(const struct btrfs_fs_info *fs_info, const char *fmt,
>  		size_t size = printk_skip_level(fmt) - fmt;
>  
>  		if (kern_level >= '0' && kern_level <= '7') {
> -			memcpy(lvl, fmt,  size);
> -			lvl[size] = '\0';
> +			strscpy(lvl, fmt, size + 1);
>  			type = logtypes[kern_level - '0'];
>  			ratelimit = &printk_limits[kern_level - '0'];
>  		}


  reply	other threads:[~2025-11-30 11:07 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-30  0:55 [PATCH] btrfs: Replace memcpy + NUL termination in _btrfs_printk Thorsten Blum
2025-11-30 11:06 ` david laight [this message]
2025-12-03 13:45   ` David Sterba
2025-12-15 20:22     ` David Sterba

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=20251130110640.21eadec5@pumpkin \
    --to=david.laight@runbox.com \
    --cc=clm@fb.com \
    --cc=dsterba@suse.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=thorsten.blum@linux.dev \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox