mv: decouple if/else-if checks using goto
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@github.com> Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> Acked-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Shaoxuan Yuan committed
Jun 30, 2022 at 10:37 UTC
7889755bae89fd792924e95d713a94578b902d11
1 file changed
+80
-59
builtin/mv.c
+80
-59
@@ -187,53 +187,68 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
187
length = strlen(src);
188
if (lstat(src, &st) < 0) {
189
/* only error if existence is expected. */
190
- if (modes[i] != SPARSE)
190
+ if (modes[i] != SPARSE) {
191
bad = _("bad source");
192
- } else if (!strncmp(src, dst, length) &&
193
- (dst[length] == 0 || dst[length] == '/')) {
192
+ goto act_on_entry;
193
+ }
194
+ }
195
+ if (!strncmp(src, dst, length) &&
196
+ (dst[length] == 0 || dst[length] == '/')) {
197
bad = _("can not move directory into itself");
195
- } else if ((src_is_dir = S_ISDIR(st.st_mode))
196
- && lstat(dst, &st) == 0)
198
+ goto act_on_entry;
199
+ }
200
+ if ((src_is_dir = S_ISDIR(st.st_mode))
201
+ && lstat(dst, &st) == 0) {
202
bad = _("cannot move directory over file");
198
- else if (src_is_dir) {
203
+ goto act_on_entry;
204
+ }
205
+ if (src_is_dir) {
206
+ int j, dst_len, n;
207
int first = cache_name_pos(src, length), last;
208
201
- if (first >= 0)
209
+ if (first >= 0) {
210
prepare_move_submodule(src, first,
211
submodule_gitfile + i);
204
- else if (index_range_of_same_dir(src, length,
205
- &first, &last) < 1)
212
+ goto act_on_entry;
213
+ } else if (index_range_of_same_dir(src, length,
214
+ &first, &last) < 1) {
215
bad = _("source directory is empty");
207
- else { /* last - first >= 1 */
208
- int j, dst_len, n;
209
-
210
- modes[i] = WORKING_DIRECTORY;
211
- n = argc + last - first;
212
- REALLOC_ARRAY(source, n);
213
- REALLOC_ARRAY(destination, n);
214
- REALLOC_ARRAY(modes, n);
215
- REALLOC_ARRAY(submodule_gitfile, n);
216
-
217
- dst = add_slash(dst);
218
- dst_len = strlen(dst);
219
-
220
- for (j = 0; j < last - first; j++) {
221
- const struct cache_entry *ce = active_cache[first + j];
222
- const char *path = ce->name;
223
- source[argc + j] = path;
224
- destination[argc + j] =
225
- prefix_path(dst, dst_len, path + length + 1);
226
- modes[argc + j] = ce_skip_worktree(ce) ? SPARSE : INDEX;
227
- submodule_gitfile[argc + j] = NULL;
228
- }
229
- argc += last - first;
216
+ goto act_on_entry;
217
}
231
- } else if (!(ce = cache_file_exists(src, length, 0))) {
218
+
219
+ /* last - first >= 1 */
220
+ modes[i] = WORKING_DIRECTORY;
221
+ n = argc + last - first;
222
+ REALLOC_ARRAY(source, n);
223
+ REALLOC_ARRAY(destination, n);
224
+ REALLOC_ARRAY(modes, n);
225
+ REALLOC_ARRAY(submodule_gitfile, n);
226
+
227
+ dst = add_slash(dst);
228
+ dst_len = strlen(dst);
229
+
230
+ for (j = 0; j < last - first; j++) {
231
+ const struct cache_entry *ce = active_cache[first + j];
232
+ const char *path = ce->name;
233
+ source[argc + j] = path;
234
+ destination[argc + j] =
235
+ prefix_path(dst, dst_len, path + length + 1);
236
+ modes[argc + j] = ce_skip_worktree(ce) ? SPARSE : INDEX;
237
+ submodule_gitfile[argc + j] = NULL;
238
+ }
239
+ argc += last - first;
240
+ goto act_on_entry;
241
+ }
242
+ if (!(ce = cache_file_exists(src, length, 0))) {
243
bad = _("not under version control");
233
- } else if (ce_stage(ce)) {
244
+ goto act_on_entry;
245
+ }
246
+ if (ce_stage(ce)) {
247
bad = _("conflicted");
235
- } else if (lstat(dst, &st) == 0 &&
236
- (!ignore_case || strcasecmp(src, dst))) {
248
+ goto act_on_entry;
249
+ }
250
+ if (lstat(dst, &st) == 0 &&
251
+ (!ignore_case || strcasecmp(src, dst))) {
252
bad = _("destination exists");
253
if (force) {
254
/*
@@ -247,34 +262,40 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
262
} else
263
bad = _("Cannot overwrite");
264
}
250
- } else if (string_list_has_string(&src_for_dst, dst))
265
+ goto act_on_entry;
266
+ }
267
+ if (string_list_has_string(&src_for_dst, dst)) {
268
bad = _("multiple sources for the same target");
252
- else if (is_dir_sep(dst[strlen(dst) - 1]))
269
+ goto act_on_entry;
270
+ }
271
+ if (is_dir_sep(dst[strlen(dst) - 1])) {
272
bad = _("destination directory does not exist");
254
- else {
255
- /*
256
- * We check if the paths are in the sparse-checkout
257
- * definition as a very final check, since that
258
- * allows us to point the user to the --sparse
259
- * option as a way to have a successful run.
260
- */
261
- if (!ignore_sparse &&
262
- !path_in_sparse_checkout(src, &the_index)) {
263
- string_list_append(&only_match_skip_worktree, src);
264
- skip_sparse = 1;
265
- }
266
- if (!ignore_sparse &&
267
- !path_in_sparse_checkout(dst, &the_index)) {
268
- string_list_append(&only_match_skip_worktree, dst);
269
- skip_sparse = 1;
270
- }
271
-
272
- if (skip_sparse)
273
- goto remove_entry;
273
+ goto act_on_entry;
274
+ }
275
275
- string_list_insert(&src_for_dst, dst);
276
+ /*
277
+ * We check if the paths are in the sparse-checkout
278
+ * definition as a very final check, since that
279
+ * allows us to point the user to the --sparse
280
+ * option as a way to have a successful run.
281
+ */
282
+ if (!ignore_sparse &&
283
+ !path_in_sparse_checkout(src, &the_index)) {
284
+ string_list_append(&only_match_skip_worktree, src);
285
+ skip_sparse = 1;
286
+ }
287
+ if (!ignore_sparse &&
288
+ !path_in_sparse_checkout(dst, &the_index)) {
289
+ string_list_append(&only_match_skip_worktree, dst);
290
+ skip_sparse = 1;
291
}
292
293
+ if (skip_sparse)
294
+ goto remove_entry;
295
+
296
+ string_list_insert(&src_for_dst, dst);
297
+
298
+act_on_entry:
299
if (!bad)
300
continue;
301
if (!ignore_errors)