| Message ID | 9fcc9f01bdab846db231b427f98fbb3e9df7c7a5.1734370092.git.anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | raid1 balancing methods | expand |
On Tue, Dec 17, 2024 at 02:13:11AM +0800, Anand Jain wrote: > Introduce the `btrfs_read_policy_to_enum` helper function to simplify the > conversion of a string read policy to its corresponding enum value. This > reduces duplication and improves code clarity in `btrfs_read_policy_store`. > The `btrfs_read_policy_store` function has been refactored to use the new > helper. > > The parameter is copied locally to allow modification, enabling the > separation of the method and its value. This prepares for the addition of > more functionality in subsequent patches. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > --- > fs/btrfs/sysfs.c | 46 ++++++++++++++++++++++++++++++++++------------ > 1 file changed, 34 insertions(+), 12 deletions(-) > > diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c > index fd3c49c6c3c5..34903e5bf8d0 100644 > --- a/fs/btrfs/sysfs.c > +++ b/fs/btrfs/sysfs.c > @@ -1307,6 +1307,30 @@ BTRFS_ATTR(, temp_fsid, btrfs_temp_fsid_show); > > static const char * const btrfs_read_policy_name[] = { "pid" }; > > +static int btrfs_read_policy_to_enum(const char *str) > +{ > + char param[32] = {'\0'}; > + int index; > + bool found = false; > + > + if (!str || strlen(str) == 0) > + return 0; > + > + strcpy(param, str); I think "str" is originated from user input. So, using strcpy can cause a buffer overflow. > + > + for (index = 0; index < BTRFS_NR_READ_POLICY; index++) { > + if (sysfs_streq(param, btrfs_read_policy_name[index])) { > + found = true; > + break; > + } > + } > + > + if (found) > + return index; > + > + return -EINVAL; We can replace the logic with sysfs_match_string(). > +} > + > static ssize_t btrfs_read_policy_show(struct kobject *kobj, > struct kobj_attribute *a, char *buf) > { > @@ -1338,21 +1362,19 @@ static ssize_t btrfs_read_policy_store(struct kobject *kobj, > const char *buf, size_t len) > { > struct btrfs_fs_devices *fs_devices = to_fs_devs(kobj); > - int i; > + int index; > > - for (i = 0; i < BTRFS_NR_READ_POLICY; i++) { > - if (sysfs_streq(buf, btrfs_read_policy_name[i])) { > - if (i != READ_ONCE(fs_devices->read_policy)) { > - WRITE_ONCE(fs_devices->read_policy, i); > - btrfs_info(fs_devices->fs_info, > - "read policy set to '%s'", > - btrfs_read_policy_name[i]); > - } > - return len; > - } > + index = btrfs_read_policy_to_enum(buf); > + if (index == -EINVAL) > + return -EINVAL; > + > + if (index != READ_ONCE(fs_devices->read_policy)) { > + WRITE_ONCE(fs_devices->read_policy, index); > + btrfs_info(fs_devices->fs_info, "read policy set to '%s'", > + btrfs_read_policy_name[index]); > } > > - return -EINVAL; > + return len; > } > BTRFS_ATTR_RW(, read_policy, btrfs_read_policy_show, btrfs_read_policy_store); > > -- > 2.47.0 >
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c index fd3c49c6c3c5..34903e5bf8d0 100644 --- a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -1307,6 +1307,30 @@ BTRFS_ATTR(, temp_fsid, btrfs_temp_fsid_show); static const char * const btrfs_read_policy_name[] = { "pid" }; +static int btrfs_read_policy_to_enum(const char *str) +{ + char param[32] = {'\0'}; + int index; + bool found = false; + + if (!str || strlen(str) == 0) + return 0; + + strcpy(param, str); + + for (index = 0; index < BTRFS_NR_READ_POLICY; index++) { + if (sysfs_streq(param, btrfs_read_policy_name[index])) { + found = true; + break; + } + } + + if (found) + return index; + + return -EINVAL; +} + static ssize_t btrfs_read_policy_show(struct kobject *kobj, struct kobj_attribute *a, char *buf) { @@ -1338,21 +1362,19 @@ static ssize_t btrfs_read_policy_store(struct kobject *kobj, const char *buf, size_t len) { struct btrfs_fs_devices *fs_devices = to_fs_devs(kobj); - int i; + int index; - for (i = 0; i < BTRFS_NR_READ_POLICY; i++) { - if (sysfs_streq(buf, btrfs_read_policy_name[i])) { - if (i != READ_ONCE(fs_devices->read_policy)) { - WRITE_ONCE(fs_devices->read_policy, i); - btrfs_info(fs_devices->fs_info, - "read policy set to '%s'", - btrfs_read_policy_name[i]); - } - return len; - } + index = btrfs_read_policy_to_enum(buf); + if (index == -EINVAL) + return -EINVAL; + + if (index != READ_ONCE(fs_devices->read_policy)) { + WRITE_ONCE(fs_devices->read_policy, index); + btrfs_info(fs_devices->fs_info, "read policy set to '%s'", + btrfs_read_policy_name[index]); } - return -EINVAL; + return len; } BTRFS_ATTR_RW(, read_policy, btrfs_read_policy_show, btrfs_read_policy_store);
Introduce the `btrfs_read_policy_to_enum` helper function to simplify the conversion of a string read policy to its corresponding enum value. This reduces duplication and improves code clarity in `btrfs_read_policy_store`. The `btrfs_read_policy_store` function has been refactored to use the new helper. The parameter is copied locally to allow modification, enabling the separation of the method and its value. This prepares for the addition of more functionality in subsequent patches. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- fs/btrfs/sysfs.c | 46 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 12 deletions(-)