Re: [PATCH] btrfs-progs: print-tree: fix chunk/block group flags output
flat view
From: Nikolay Borisov <hidden>
Date: 2021-10-12 10:35:13
<snip>
quoted hunk ↗ jump to hunk
Signed-off-by: Qu Wenruo <redacted> --- kernel-shared/print-tree.c | 47 +++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 18 deletions(-)diff --git a/kernel-shared/print-tree.c b/kernel-shared/print-tree.c index 67b654e6d2d5..39655590272e 100644 --- a/kernel-shared/print-tree.c +++ b/kernel-shared/print-tree.c@@ -159,40 +159,51 @@ static void print_inode_ref_item(struct extent_buffer *eb, u32 size, } } -/* Caller should ensure sizeof(*ret)>=21 "DATA|METADATA|RAID10" */ +/* The minimal length for the string buffer of block group/chunk flags */ +#define BG_FLAG_STRING_LEN 64 + static void bg_flags_to_str(u64 flags, char *ret) { int empty = 1; + char profile[BG_FLAG_STRING_LEN] = {}; const char *name; + ret[0] = '\0'; if (flags & BTRFS_BLOCK_GROUP_DATA) { empty = 0; - strcpy(ret, "DATA"); + strncpy(ret, "DATA", BG_FLAG_STRING_LEN);
I find using strncpy rather odd, it guarantees it will copy num characters, and if source is smaller than dest, it will overwrite the rest with 0. So what happens is you are copying 4 chars here, and writing 60 zeros. Frankly I think it's better to use snprintf(ret, BG_FLAG_STRING_LEN, "DATA");
quoted hunk ↗ jump to hunk
} if (flags & BTRFS_BLOCK_GROUP_METADATA) { if (!empty) - strcat(ret, "|"); - strcat(ret, "METADATA"); + strncat(ret, "|", BG_FLAG_STRING_LEN); + strncat(ret, "METADATA", BG_FLAG_STRING_LEN); } if (flags & BTRFS_BLOCK_GROUP_SYSTEM) { if (!empty) - strcat(ret, "|"); - strcat(ret, "SYSTEM"); + strncat(ret, "|", BG_FLAG_STRING_LEN); + strncat(ret, "SYSTEM", BG_FLAG_STRING_LEN); } - strcat(ret, "|"); name = btrfs_bg_type_to_raid_name(flags); if (!name) { - strcat(ret, "UNKNOWN"); + snprintf(profile, BG_FLAG_STRING_LEN, "UNKNOWN.0x%llx", + flags & BTRFS_BLOCK_GROUP_PROFILE_MASK); } else { - char buf[32]; - char *tmp = buf; + int i; - strcpy(buf, name); - while (*tmp) { - *tmp = toupper(*tmp); - tmp++; - } - strcpy(ret, buf); + /* + * Special handing for SINGLE profile, we don't output "SINGLE" + * for SINGLE profile, since there is no such bit for it. + * Thus here we only fill @profile if it's not single. + */ + if (strncmp(name, "single", strlen("single")) != 0) + strncpy(profile, name, BG_FLAG_STRING_LEN); + + for (i = 0; i < BG_FLAG_STRING_LEN && profile[i]; i++)
nit: It's guaranteed that the profile is shorted than BG_FLAG_STRING_LEN, then this check can simply be profile[i] without the i/BG_FLAG_STRING_LEN constant comparison.
quoted hunk ↗ jump to hunk
+ profile[i] = toupper(profile[i]); + } + if (profile[0]) { + strncat(ret, "|", BG_FLAG_STRING_LEN); + strncat(ret, profile, BG_FLAG_STRING_LEN); }
This if can really be put in the above 'else' branch and eliminate the check altogether.
}
<snip>