| Message ID | 1396948562-16653-1-git-send-email-quwenruo@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
I expect btrfs_scan_kernel() would go away in the long run, however below fix has a problem - when there is no search item you would return fail (1) always. Since btrfs_scan_kernel() has limited usage as you mention as well. So I will leave it to David to decide. Thanks, Anand On 08/04/2014 17:16, Qu Wenruo wrote: > btrfs_scan_kernel() is only used in 'btrfs fi show' but returns wrong > return value. > When search parameter is passed, it will never return 0 even the search > can be matched. > > This patch will change the whatever strange logic to a more easy to > understand one using 'found' var. > > Reported-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com> > Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> > Cc: Anand Jain <anand.jain@oracle.com> > --- > cmds-filesystem.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/cmds-filesystem.c b/cmds-filesystem.c > index b81768b..49d3aa7 100644 > --- a/cmds-filesystem.c > +++ b/cmds-filesystem.c > @@ -430,6 +430,7 @@ static int btrfs_scan_kernel_v2(void *search) > static int btrfs_scan_kernel(void *search) > { > int ret = 0, fd; > + int found = 0; > FILE *f; > struct mntent *mnt; > struct btrfs_ioctl_fs_info_args fs_info_arg; > @@ -452,7 +453,6 @@ static int btrfs_scan_kernel(void *search) > > if (get_label_mounted(mnt->mnt_dir, label)) { > kfree(dev_info_arg); > - ret = 1; > goto out; > } > if (search && !match_search_item_kernel(fs_info_arg.fsid, > @@ -467,19 +467,16 @@ static int btrfs_scan_kernel(void *search) > space_info_arg, label, mnt->mnt_dir); > kfree(space_info_arg); > memset(label, 0, sizeof(label)); > + found = 1; > } > if (fd != -1) > close(fd); > kfree(dev_info_arg); > - if (search) > - ret = 0; > } > - if (search) > - ret = 1; > > out: > endmntent(f); > - return ret; > + return !found; > } > > static int dev_to_fsid(char *dev, __u8 *fsid) > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Nope, the fix can also deal with no search parameter. If any btrfs filesystem can be found and printed, found will be set to 1 which considered as found, then 0 will be returned. Thanks, Qu ? 2014?04?09? 10:09, Anand Jain ??: > > I expect btrfs_scan_kernel() would go away in the long run, > however below fix has a problem - when there is no search item > you would return fail (1) always. Since btrfs_scan_kernel() has > limited usage as you mention as well. So I will leave it to David > to decide. > > Thanks, Anand > > On 08/04/2014 17:16, Qu Wenruo wrote: >> btrfs_scan_kernel() is only used in 'btrfs fi show' but returns wrong >> return value. >> When search parameter is passed, it will never return 0 even the search >> can be matched. >> >> This patch will change the whatever strange logic to a more easy to >> understand one using 'found' var. >> >> Reported-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com> >> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> >> Cc: Anand Jain <anand.jain@oracle.com> >> --- >> cmds-filesystem.c | 9 +++------ >> 1 file changed, 3 insertions(+), 6 deletions(-) >> >> diff --git a/cmds-filesystem.c b/cmds-filesystem.c >> index b81768b..49d3aa7 100644 >> --- a/cmds-filesystem.c >> +++ b/cmds-filesystem.c >> @@ -430,6 +430,7 @@ static int btrfs_scan_kernel_v2(void *search) >> static int btrfs_scan_kernel(void *search) >> { >> int ret = 0, fd; >> + int found = 0; >> FILE *f; >> struct mntent *mnt; >> struct btrfs_ioctl_fs_info_args fs_info_arg; >> @@ -452,7 +453,6 @@ static int btrfs_scan_kernel(void *search) >> >> if (get_label_mounted(mnt->mnt_dir, label)) { >> kfree(dev_info_arg); >> - ret = 1; >> goto out; >> } >> if (search && !match_search_item_kernel(fs_info_arg.fsid, >> @@ -467,19 +467,16 @@ static int btrfs_scan_kernel(void *search) >> space_info_arg, label, mnt->mnt_dir); >> kfree(space_info_arg); >> memset(label, 0, sizeof(label)); >> + found = 1; >> } >> if (fd != -1) >> close(fd); >> kfree(dev_info_arg); >> - if (search) >> - ret = 0; >> } >> - if (search) >> - ret = 1; >> >> out: >> endmntent(f); >> - return ret; >> + return !found; >> } >> >> static int dev_to_fsid(char *dev, __u8 *fsid) >> -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/04/2014 10:18, Qu Wenruo wrote: > Nope, the fix can also deal with no search parameter. > If any btrfs filesystem can be found and printed, found will be set to 1 > which considered as found, > then 0 will be returned. Oh yes. You are right. Thanks, Anand > Thanks, > Qu > ? 2014?04?09? 10:09, Anand Jain ??: >> >> I expect btrfs_scan_kernel() would go away in the long run, >> however below fix has a problem - when there is no search item >> you would return fail (1) always. Since btrfs_scan_kernel() has >> limited usage as you mention as well. So I will leave it to David >> to decide. >> >> Thanks, Anand >> >> On 08/04/2014 17:16, Qu Wenruo wrote: >>> btrfs_scan_kernel() is only used in 'btrfs fi show' but returns wrong >>> return value. >>> When search parameter is passed, it will never return 0 even the search >>> can be matched. >>> >>> This patch will change the whatever strange logic to a more easy to >>> understand one using 'found' var. >>> >>> Reported-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com> >>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> >>> Cc: Anand Jain <anand.jain@oracle.com> >>> --- >>> cmds-filesystem.c | 9 +++------ >>> 1 file changed, 3 insertions(+), 6 deletions(-) >>> >>> diff --git a/cmds-filesystem.c b/cmds-filesystem.c >>> index b81768b..49d3aa7 100644 >>> --- a/cmds-filesystem.c >>> +++ b/cmds-filesystem.c >>> @@ -430,6 +430,7 @@ static int btrfs_scan_kernel_v2(void *search) >>> static int btrfs_scan_kernel(void *search) >>> { >>> int ret = 0, fd; >>> + int found = 0; >>> FILE *f; >>> struct mntent *mnt; >>> struct btrfs_ioctl_fs_info_args fs_info_arg; >>> @@ -452,7 +453,6 @@ static int btrfs_scan_kernel(void *search) >>> >>> if (get_label_mounted(mnt->mnt_dir, label)) { >>> kfree(dev_info_arg); >>> - ret = 1; >>> goto out; >>> } >>> if (search && !match_search_item_kernel(fs_info_arg.fsid, >>> @@ -467,19 +467,16 @@ static int btrfs_scan_kernel(void *search) >>> space_info_arg, label, mnt->mnt_dir); >>> kfree(space_info_arg); >>> memset(label, 0, sizeof(label)); >>> + found = 1; >>> } >>> if (fd != -1) >>> close(fd); >>> kfree(dev_info_arg); >>> - if (search) >>> - ret = 0; >>> } >>> - if (search) >>> - ret = 1; >>> >>> out: >>> endmntent(f); >>> - return ret; >>> + return !found; >>> } >>> >>> static int dev_to_fsid(char *dev, __u8 *fsid) >>> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/cmds-filesystem.c b/cmds-filesystem.c index b81768b..49d3aa7 100644 --- a/cmds-filesystem.c +++ b/cmds-filesystem.c @@ -430,6 +430,7 @@ static int btrfs_scan_kernel_v2(void *search) static int btrfs_scan_kernel(void *search) { int ret = 0, fd; + int found = 0; FILE *f; struct mntent *mnt; struct btrfs_ioctl_fs_info_args fs_info_arg; @@ -452,7 +453,6 @@ static int btrfs_scan_kernel(void *search) if (get_label_mounted(mnt->mnt_dir, label)) { kfree(dev_info_arg); - ret = 1; goto out; } if (search && !match_search_item_kernel(fs_info_arg.fsid, @@ -467,19 +467,16 @@ static int btrfs_scan_kernel(void *search) space_info_arg, label, mnt->mnt_dir); kfree(space_info_arg); memset(label, 0, sizeof(label)); + found = 1; } if (fd != -1) close(fd); kfree(dev_info_arg); - if (search) - ret = 0; } - if (search) - ret = 1; out: endmntent(f); - return ret; + return !found; } static int dev_to_fsid(char *dev, __u8 *fsid)
btrfs_scan_kernel() is only used in 'btrfs fi show' but returns wrong return value. When search parameter is passed, it will never return 0 even the search can be matched. This patch will change the whatever strange logic to a more easy to understand one using 'found' var. Reported-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> Cc: Anand Jain <anand.jain@oracle.com> --- cmds-filesystem.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)