Re: [PATCH v2] btrfs: use strstarts() and str_has_prefix() for prefix checks

[Qu Wenruo <wqu@suse.com>]

在 2026/9/26 21:52, George Hu 写道:
> Replace strncmp() prefix checks with strstarts() when only a boolean result
> is needed, and with str_has_prefix() when the prefix length is needed after
> a successful match.
> 
> This makes prefix checks clearer and more concise.
> 
> Signed-off-by: George Hu <integral@archlinux.org>
> ---
> V1 -> V2:
>   - Avoid replacing strncmp() with strstarts() when the parameter is
>     not NUL terminated
>   - Revise subject
> 
>   fs/btrfs/super.c | 5 ++---
>   fs/btrfs/xattr.c | 3 +--
>   2 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index ddb620ac241b..287cdc45bef8 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -263,10 +263,9 @@ static const struct fs_parameter_spec btrfs_fs_parameters[] = {
>   
>   static bool btrfs_match_compress_type(const char *string, const char *type, bool may_have_level)
>   {
> -	const int len = strlen(type);
> +	const size_t len = str_has_prefix(string, type);
>   
> -	return (strncmp(string, type, len) == 0) &&
> -		((may_have_level && string[len] == ':') || string[len] == '\0');
> +	return len && ((may_have_level && string[len] == ':') || string[len] == '\0');

For this call site, both strings are NUL terminated so there is no 
problem about accessing beyond boundary.

But to be honest, I don't think this improves the readability.

I still have to check what str_has_prefix() really returns to understand 
the code, as the function is not a common helper and just by the name I 
don't get what if it will return.

>   }
>   
>   static int btrfs_parse_compress(struct btrfs_fs_context *ctx,
> diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c
> index ab55d10bd71f..9011d8096e2d 100644
> --- a/fs/btrfs/xattr.c
> +++ b/fs/btrfs/xattr.c
> @@ -239,8 +239,7 @@ int btrfs_setxattr_trans(struct inode *inode, const char *name,
>   		 * block_rsv of the handle and trigger a warning for the start
>   		 * case.
>   		 */
> -		ASSERT(strncmp(name, XATTR_SECURITY_PREFIX,
> -			       XATTR_SECURITY_PREFIX_LEN) == 0);
> +		ASSERT(strstarts(name, XATTR_SECURITY_PREFIX));

@name is not NUL-terminated, thus even the old code is not safe.

The only thing saving us here is that we should have a security XATTR 
passed in, thus the name must have the "security." prefix and not cause 
access-beyond-boundary.

Again, I do not think it's a good idea to use strstarts() with any 
non-NUL-terminated strings.


>   		trans = current->journal_info;
>   	}
>