Re: [PATCH v2] xfs: Replace strncpy with memcpy

[Carlos Maiolino]

Hi.

On Sun, Aug 17, 2025 at 12:50:41PM -0300, Marcelo Moreira wrote:
> Following a suggestion from Dave and everyone who contributed to v1, this
> changes modernizes the code by aligning it with current kernel best practices.
> It improves code clarity and consistency, as strncpy is deprecated as explained
> in Documentation/process/deprecated.rst. Furthermore, this change was tested
> by xfstests

> and as it was not an easy task I decided to document on my blog
> the step by step of how I did it https://meritissimo1.com/blog/2-xfs-tests :).

The above line does not belong to the commit description. I'm glad
you've tested everything as we suggested and got to the point to run
xfstests which indeed is not a single-click button. But the patch
description is not a place to document it.

> 
> This change does not alter the functionality or introduce any behavioral
> changes.

^ This should be in the description...

> 
> Changes include:
>  - Replace strncpy with memcpy.

^ This is unnecessary. It's a plain copy/paste from the subject, no need
to write yet again.

> 
> ---

^ Keep a single --- in the patch... This is used as metadata, everything
  below the first --- is ignored by git.

> Changelog:
> 
> Changes since v1:
> - Replace strncpy with memcpy instead of strscpy.
> - The change was tested using xfstests.
> 
> Link to v1: https://lore.kernel.org/linux-kernel-mentees/CAPZ3m_jXwp1FfsvtR2s3nwATT3fER=Mc6qj+GzKuUhY5tjQFNQ@xxxxxxxxxxxxxx/T/#t
> 

^ All those Changelog metadata should be below the ---, so they don't
get into the commit message, but...

> Suggested-by: Dave Chinner <david@xxxxxxxxxxxxx>
> Signed-off-by: Marcelo Moreira <marcelomoreira1905@xxxxxxxxx>

^ Those should be before the '---' otherwise, as I mentioned, git will
ignore those.

> ---
>  fs/xfs/scrub/symlink_repair.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/scrub/symlink_repair.c b/fs/xfs/scrub/symlink_repair.c
> index 953ce7be78dc..5902398185a8 100644
> --- a/fs/xfs/scrub/symlink_repair.c
> +++ b/fs/xfs/scrub/symlink_repair.c
> @@ -185,7 +185,7 @@ xrep_symlink_salvage_inline(
>  		return 0;
> 
>  	nr = min(XFS_SYMLINK_MAXLEN, xfs_inode_data_fork_size(ip));
> -	strncpy(target_buf, ifp->if_data, nr);
> +	memcpy(target_buf, ifp->if_data, nr);
>  	return nr;

The change looks fine. Once you fix the above points:

Reviewed-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>

Cheers,
Carlos

>  }
> 
> --
> 2.50.1
> 
>