On Tue, Nov 07, 2017 at 10:24:30AM +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@xxxxxxxxxxxxxx>
> ---
> cmds-filesystem.c | 46 +++++++++++++++++++++++++---------------------
> 1 file changed, 25 insertions(+), 21 deletions(-)
>
> diff --git a/cmds-filesystem.c b/cmds-filesystem.c
> index 7728430f16a1..0893a44f28fe 100644
> --- a/cmds-filesystem.c
> +++ b/cmds-filesystem.c
> @@ -1029,29 +1029,27 @@ 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 = fd;
The fd will be -1, we might need to save the last error, though it's not
directly used (yet).
> + 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;
> + goto next;
Similar here.
> }
> 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;
So it's consistent with that one.
> + goto next;
> }
> if (recursive && S_ISDIR(st.st_mode)) {
> ret = nftw(argv[i], defrag_callback, 10,
> FTW_MOUNT | FTW_PHYS);
> if (ret == ENOTTY)
> - exit(1);
> + break;
Please split this change, it changes the logic and should be properly
described in the changelog.
Otherwise ok.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
