Previous if/else-if chain are highly nested and hard to develop/extend. Refactor to decouple this if/else-if chain by using goto to jump ahead. Suggested-by: Derrick Stolee <derrickstolee@xxxxxxxxxx> Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@xxxxxxxxx> --- builtin/mv.c | 139 +++++++++++++++++++++++++++++---------------------- 1 file changed, 80 insertions(+), 59 deletions(-) diff --git a/builtin/mv.c b/builtin/mv.c index 83a465ba83..1ca2c21da8 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -186,53 +186,68 @@ int cmd_mv(int argc, const char **argv, const char *prefix) length = strlen(src); if (lstat(src, &st) < 0) { /* only error if existence is expected. */ - if (modes[i] != SPARSE) + if (modes[i] != SPARSE) { bad = _("bad source"); - } else if (!strncmp(src, dst, length) && - (dst[length] == 0 || dst[length] == '/')) { + goto act_on_entry; + } + } + if (!strncmp(src, dst, length) && + (dst[length] == 0 || dst[length] == '/')) { bad = _("can not move directory into itself"); - } else if ((src_is_dir = S_ISDIR(st.st_mode)) - && lstat(dst, &st) == 0) + goto act_on_entry; + } + if ((src_is_dir = S_ISDIR(st.st_mode)) + && lstat(dst, &st) == 0) { bad = _("cannot move directory over file"); - else if (src_is_dir) { + goto act_on_entry; + } + if (src_is_dir) { + int j, dst_len, n; int first = cache_name_pos(src, length), last; - if (first >= 0) + if (first >= 0) { prepare_move_submodule(src, first, submodule_gitfile + i); - else if (index_range_of_same_dir(src, length, - &first, &last) < 1) + goto act_on_entry; + } else if (index_range_of_same_dir(src, length, + &first, &last) < 1) { bad = _("source directory is empty"); - else { /* last - first >= 1 */ - int j, dst_len, n; - - modes[i] = WORKING_DIRECTORY; - n = argc + last - first; - REALLOC_ARRAY(source, n); - REALLOC_ARRAY(destination, n); - REALLOC_ARRAY(modes, n); - REALLOC_ARRAY(submodule_gitfile, n); - - dst = add_slash(dst); - dst_len = strlen(dst); - - for (j = 0; j < last - first; j++) { - const struct cache_entry *ce = active_cache[first + j]; - const char *path = ce->name; - source[argc + j] = path; - destination[argc + j] = - prefix_path(dst, dst_len, path + length + 1); - modes[argc + j] = ce_skip_worktree(ce) ? SPARSE : INDEX; - submodule_gitfile[argc + j] = NULL; - } - argc += last - first; + goto act_on_entry; } - } else if (!(ce = cache_file_exists(src, length, 0))) { + + /* last - first >= 1 */ + modes[i] = WORKING_DIRECTORY; + n = argc + last - first; + REALLOC_ARRAY(source, n); + REALLOC_ARRAY(destination, n); + REALLOC_ARRAY(modes, n); + REALLOC_ARRAY(submodule_gitfile, n); + + dst = add_slash(dst); + dst_len = strlen(dst); + + for (j = 0; j < last - first; j++) { + const struct cache_entry *ce = active_cache[first + j]; + const char *path = ce->name; + source[argc + j] = path; + destination[argc + j] = + prefix_path(dst, dst_len, path + length + 1); + modes[argc + j] = ce_skip_worktree(ce) ? SPARSE : INDEX; + submodule_gitfile[argc + j] = NULL; + } + argc += last - first; + goto act_on_entry; + } + if (!(ce = cache_file_exists(src, length, 0))) { bad = _("not under version control"); - } else if (ce_stage(ce)) { + goto act_on_entry; + } + if (ce_stage(ce)) { bad = _("conflicted"); - } else if (lstat(dst, &st) == 0 && - (!ignore_case || strcasecmp(src, dst))) { + goto act_on_entry; + } + if (lstat(dst, &st) == 0 && + (!ignore_case || strcasecmp(src, dst))) { bad = _("destination exists"); if (force) { /* @@ -246,34 +261,40 @@ int cmd_mv(int argc, const char **argv, const char *prefix) } else bad = _("Cannot overwrite"); } - } else if (string_list_has_string(&src_for_dst, dst)) + goto act_on_entry; + } + if (string_list_has_string(&src_for_dst, dst)) { bad = _("multiple sources for the same target"); - else if (is_dir_sep(dst[strlen(dst) - 1])) + goto act_on_entry; + } + if (is_dir_sep(dst[strlen(dst) - 1])) { bad = _("destination directory does not exist"); - else { - /* - * We check if the paths are in the sparse-checkout - * definition as a very final check, since that - * allows us to point the user to the --sparse - * option as a way to have a successful run. - */ - if (!ignore_sparse && - !path_in_sparse_checkout(src, &the_index)) { - string_list_append(&only_match_skip_worktree, src); - skip_sparse = 1; - } - if (!ignore_sparse && - !path_in_sparse_checkout(dst, &the_index)) { - string_list_append(&only_match_skip_worktree, dst); - skip_sparse = 1; - } - - if (skip_sparse) - goto remove_entry; + goto act_on_entry; + } - string_list_insert(&src_for_dst, dst); + /* + * We check if the paths are in the sparse-checkout + * definition as a very final check, since that + * allows us to point the user to the --sparse + * option as a way to have a successful run. + */ + if (!ignore_sparse && + !path_in_sparse_checkout(src, &the_index)) { + string_list_append(&only_match_skip_worktree, src); + skip_sparse = 1; + } + if (!ignore_sparse && + !path_in_sparse_checkout(dst, &the_index)) { + string_list_append(&only_match_skip_worktree, dst); + skip_sparse = 1; } + if (skip_sparse) + goto remove_entry; + + string_list_insert(&src_for_dst, dst); + +act_on_entry: if (!bad) continue; if (!ignore_errors) -- 2.35.1