| Message ID | 20171128091450.21789-1-suy.fnst@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Nov 28, 2017 at 05:14:48PM +0800, Su Yue wrote: > In function cmd_filesystem_defrag(), lines of code for error handling > are duplicate and hard to expand in further. > > Create a jump label for errors. > > Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> Applied, thanks. -- 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 2017年11月28日 17:14, Su Yue wrote: > In function cmd_filesystem_defrag(), lines of code for error handling > are duplicate and hard to expand in further. > > Create a jump label for errors. > > Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> > --- > Changelog: > v2: Record -errno to return value if open() and fstat() failed. > Move change "do no exit if defrag range ioctl is unsupported" > to another patch. > Suggested by David. > --- > cmds-filesystem.c | 44 ++++++++++++++++++++++++-------------------- > 1 file changed, 24 insertions(+), 20 deletions(-) > > diff --git a/cmds-filesystem.c b/cmds-filesystem.c > index 7728430f16a1..17d399d58adf 100644 > --- a/cmds-filesystem.c > +++ b/cmds-filesystem.c > @@ -1029,23 +1029,22 @@ static int cmd_filesystem_defrag(int argc, char **argv) > if (fd < 0) { > error("cannot open %s: %s", argv[i], > strerror(errno)); > - defrag_global_errors++; > - close_file_or_dir(fd, dirstream); > - continue; > + ret = -errno; > + goto next; > } > - if (fstat(fd, &st)) { > + > + ret = fstat(fd, &st); > + if (ret) { > error("failed to stat %s: %s", > argv[i], strerror(errno)); > - defrag_global_errors++; > - close_file_or_dir(fd, dirstream); > - continue; > + ret = -errno; > + goto next; > } > if (!(S_ISDIR(st.st_mode) || S_ISREG(st.st_mode))) { > error("%s is not a directory or a regular file", > argv[i]); > - defrag_global_errors++; > - close_file_or_dir(fd, dirstream); > - continue; > + ret = -EINVAL; > + goto next; Here, unlike the ftw callback, which ignore non-regular file, here we count them as error. So if there is some especial file in dir (use "example_dir" as an example), then # btrfs fi defrag -r example_dir will return no error. While # btrfs fi defrag -r example_dir/* Will return error. IIRC such inconsistent behavior is a bigger problem, and reusing the defrag_callback() will not only makes the behavior consistent, but also simplify the loop. Thanks, Qu > } > if (recursive && S_ISDIR(st.st_mode)) { > ret = nftw(argv[i], defrag_callback, 10, > @@ -1060,20 +1059,25 @@ static int cmd_filesystem_defrag(int argc, char **argv) > ret = ioctl(fd, BTRFS_IOC_DEFRAG_RANGE, > &defrag_global_range); > defrag_err = errno; > - } > - close_file_or_dir(fd, dirstream); > - if (ret && defrag_err == ENOTTY) { > - error( > + if (ret && defrag_err == ENOTTY) { > + error( > "defrag range ioctl not supported in this kernel version, 2.6.33 and newer is required"); > - defrag_global_errors++; > - break; > + defrag_global_errors++; > + close_file_or_dir(fd, dirstream); > + break; > + } > + if (ret) { > + error("defrag failed on %s: %s", argv[i], > + strerror(defrag_err)); > + goto next; > + } > } > - if (ret) { > - error("defrag failed on %s: %s", argv[i], > - strerror(defrag_err)); > +next: > + if (ret) > defrag_global_errors++; > - } > + close_file_or_dir(fd, dirstream); > } > + > if (defrag_global_errors) > fprintf(stderr, "total %d failures\n", defrag_global_errors); > >
diff --git a/cmds-filesystem.c b/cmds-filesystem.c index 7728430f16a1..17d399d58adf 100644 --- a/cmds-filesystem.c +++ b/cmds-filesystem.c @@ -1029,23 +1029,22 @@ static int cmd_filesystem_defrag(int argc, char **argv) if (fd < 0) { error("cannot open %s: %s", argv[i], strerror(errno)); - defrag_global_errors++; - close_file_or_dir(fd, dirstream); - continue; + ret = -errno; + goto next; } - if (fstat(fd, &st)) { + + ret = fstat(fd, &st); + if (ret) { error("failed to stat %s: %s", argv[i], strerror(errno)); - defrag_global_errors++; - close_file_or_dir(fd, dirstream); - continue; + ret = -errno; + goto next; } if (!(S_ISDIR(st.st_mode) || S_ISREG(st.st_mode))) { error("%s is not a directory or a regular file", argv[i]); - defrag_global_errors++; - close_file_or_dir(fd, dirstream); - continue; + ret = -EINVAL; + goto next; } if (recursive && S_ISDIR(st.st_mode)) { ret = nftw(argv[i], defrag_callback, 10, @@ -1060,20 +1059,25 @@ static int cmd_filesystem_defrag(int argc, char **argv) ret = ioctl(fd, BTRFS_IOC_DEFRAG_RANGE, &defrag_global_range); defrag_err = errno; - } - close_file_or_dir(fd, dirstream); - if (ret && defrag_err == ENOTTY) { - error( + if (ret && defrag_err == ENOTTY) { + error( "defrag range ioctl not supported in this kernel version, 2.6.33 and newer is required"); - defrag_global_errors++; - break; + defrag_global_errors++; + close_file_or_dir(fd, dirstream); + break; + } + if (ret) { + error("defrag failed on %s: %s", argv[i], + strerror(defrag_err)); + goto next; + } } - if (ret) { - error("defrag failed on %s: %s", argv[i], - strerror(defrag_err)); +next: + if (ret) defrag_global_errors++; - } + close_file_or_dir(fd, dirstream); } + if (defrag_global_errors) fprintf(stderr, "total %d failures\n", defrag_global_errors);
In function cmd_filesystem_defrag(), lines of code for error handling are duplicate and hard to expand in further. Create a jump label for errors. Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> --- Changelog: v2: Record -errno to return value if open() and fstat() failed. Move change "do no exit if defrag range ioctl is unsupported" to another patch. Suggested by David. --- cmds-filesystem.c | 44 ++++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 20 deletions(-)