Re: [PATCH] btrfs: use helper functions to check if a string has a given prefix

[Qu Wenruo <wqu@suse.com>]

在 2026/9/26 20:06, 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>
> ---
>   fs/btrfs/compression.c | 15 ++++-----------
>   fs/btrfs/compression.h |  2 +-
>   fs/btrfs/props.c       |  2 +-
>   fs/btrfs/super.c       |  5 ++---
>   fs/btrfs/xattr.c       |  3 +--
>   5 files changed, 9 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index c62b5148d5ac..82d0413e6236 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -70,19 +70,12 @@ static struct compressed_bio *alloc_compressed_bio(struct btrfs_inode *inode,
>   	return to_compressed_bio(bbio);
>   }
>   
> -bool btrfs_compress_is_valid_type(const char *str, size_t len)
> +bool btrfs_compress_is_valid_type(const char *str)
>   {
> -	int i;
> -
> -	for (i = 1; i < ARRAY_SIZE(btrfs_compress_types); i++) {
> -		size_t comp_len = strlen(btrfs_compress_types[i]);
> -
> -		if (len < comp_len)
> -			continue;
> -
> -		if (!strncmp(btrfs_compress_types[i], str, comp_len))
> +	for (int i = 1; i < ARRAY_SIZE(btrfs_compress_types); i++)
> +		if (strstarts(str, btrfs_compress_types[i]))

strstarts() is using strlen(btrfs_compress_types[i]) to do the string 
comparison, but the old code will skip the comparison completely, to 
avoid accessing memory after @len.

Since @str is not NUL terminated, this is very dangerous.

To be honest, I didn't see strstarts() provides any readability benefit 
unless the parameter is NUL terminated.


>   			return true;
> -	}
> +
>   	return false;
>   }
>   
> diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
> index 1022dc53ec51..1d8e12fcd488 100644
> --- a/fs/btrfs/compression.h
> +++ b/fs/btrfs/compression.h
> @@ -131,7 +131,7 @@ extern const struct btrfs_compress_levels btrfs_lzo_compress;
>   extern const struct btrfs_compress_levels btrfs_zstd_compress;
>   
>   const char* btrfs_compress_type2str(enum btrfs_compression_type type);
> -bool btrfs_compress_is_valid_type(const char *str, size_t len);
> +bool btrfs_compress_is_valid_type(const char *str);
>   
>   int btrfs_compress_heuristic(struct btrfs_inode *inode, u64 start, u64 end);
>   
> diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
> index bb77d46376d4..c33930c58802 100644
> --- a/fs/btrfs/props.c
> +++ b/fs/btrfs/props.c
> @@ -301,7 +301,7 @@ static int prop_compression_validate(const struct btrfs_inode *inode,
>   	if (!value)
>   		return 0;
>   
> -	if (btrfs_compress_is_valid_type(value, len))
> +	if (btrfs_compress_is_valid_type(value))
>   		return 0;
>   
>   	if ((len == 2 && strncmp("no", value, 2) == 0) ||
> 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');
>   }
>   
>   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));
>   		trans = current->journal_info;
>   	}
>