builtin/mv: refactor `add_slash()` to always return allocated strings
The `add_slash()` function will only conditionally return an allocated string when the passed-in string did not yet have a trailing slash. This makes the memory ownership harder to track than really necessary. It's dubious whether this optimization really buys us all that much. The number of times we execute this function is bounded by the number of arguments to git-mv(1), so in the typical case we may end up saving an allocation or two. Simplify the code to unconditionally return allocated strings. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
May 27, 2024 at 13:47 UTC
3d231f7b8236787252cb336878e3ace75c1df545
1 file changed
+20
-18
builtin/mv.c
+20
-18
@@ -76,7 +76,7 @@ static const char **internal_prefix_pathspec(const char *prefix,
76
return result;
77
}
78
79
-static const char *add_slash(const char *path)
79
+static char *add_slash(const char *path)
80
{
81
size_t len = strlen(path);
82
if (len && path[len - 1] != '/') {
@@ -86,7 +86,7 @@ static const char *add_slash(const char *path)
86
with_slash[len] = 0;
87
return with_slash;
88
}
89
- return path;
89
+ return xstrdup(path);
90
}
91
92
#define SUBMODULE_WITH_GITDIR ((const char *)1)
@@ -111,7 +111,7 @@ static void prepare_move_submodule(const char *src, int first,
111
static int index_range_of_same_dir(const char *src, int length,
112
int *first_p, int *last_p)
113
{
114
- const char *src_w_slash = add_slash(src);
114
+ char *src_w_slash = add_slash(src);
115
int first, last, len_w_slash = length + 1;
116
117
first = index_name_pos(the_repository->index, src_w_slash, len_w_slash);
@@ -124,8 +124,8 @@ static int index_range_of_same_dir(const char *src, int length,
124
if (strncmp(path, src_w_slash, len_w_slash))
125
break;
126
}
127
- if (src_w_slash != src)
128
- free((char *)src_w_slash);
127
+
128
+ free(src_w_slash);
129
*first_p = first;
130
*last_p = last;
131
return last - first;
@@ -141,7 +141,7 @@ static int index_range_of_same_dir(const char *src, int length,
141
static int empty_dir_has_sparse_contents(const char *name)
142
{
143
int ret = 0;
144
- const char *with_slash = add_slash(name);
144
+ char *with_slash = add_slash(name);
145
int length = strlen(with_slash);
146
147
int pos = index_name_pos(the_repository->index, with_slash, length);
@@ -159,8 +159,7 @@ static int empty_dir_has_sparse_contents(const char *name)
159
}
160
161
free_return:
162
- if (with_slash != name)
163
- free((char *)with_slash);
162
+ free(with_slash);
163
return ret;
164
}
165
@@ -178,7 +177,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
177
OPT_END(),
178
};
179
const char **source, **destination, **dest_path, **submodule_gitfile;
181
- const char *dst_w_slash;
180
+ char *dst_w_slash = NULL;
181
const char **src_dir = NULL;
182
int src_dir_nr = 0, src_dir_alloc = 0;
183
struct strbuf a_src_dir = STRBUF_INIT;
@@ -243,10 +242,6 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
242
dst_mode = SPARSE;
243
}
244
}
246
- if (dst_w_slash != dest_path[0]) {
247
- free((char *)dst_w_slash);
248
- dst_w_slash = NULL;
249
- }
245
246
/* Checking */
247
for (i = 0; i < argc; i++) {
@@ -265,12 +260,14 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
260
261
pos = index_name_pos(the_repository->index, src, length);
262
if (pos < 0) {
268
- const char *src_w_slash = add_slash(src);
263
+ char *src_w_slash = add_slash(src);
264
if (!path_in_sparse_checkout(src_w_slash, the_repository->index) &&
265
empty_dir_has_sparse_contents(src)) {
266
+ free(src_w_slash);
267
modes[i] |= SKIP_WORKTREE_DIR;
268
goto dir_check;
269
}
270
+ free(src_w_slash);
271
/* only error if existence is expected. */
272
if (!(modes[i] & SPARSE))
273
bad = _("bad source");
@@ -310,7 +307,9 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
307
308
dir_check:
309
if (S_ISDIR(st.st_mode)) {
313
- int j, dst_len, n;
310
+ char *dst_with_slash;
311
+ size_t dst_with_slash_len;
312
+ int j, n;
313
int first = index_name_pos(the_repository->index, src, length), last;
314
315
if (first >= 0) {
@@ -335,19 +334,21 @@ dir_check:
334
REALLOC_ARRAY(modes, n);
335
REALLOC_ARRAY(submodule_gitfile, n);
336
338
- dst = add_slash(dst);
339
- dst_len = strlen(dst);
337
+ dst_with_slash = add_slash(dst);
338
+ dst_with_slash_len = strlen(dst_with_slash);
339
340
for (j = 0; j < last - first; j++) {
341
const struct cache_entry *ce = the_repository->index->cache[first + j];
342
const char *path = ce->name;
343
source[argc + j] = path;
344
destination[argc + j] =
346
- prefix_path(dst, dst_len, path + length + 1);
345
+ prefix_path(dst_with_slash, dst_with_slash_len, path + length + 1);
346
memset(modes + argc + j, 0, sizeof(enum update_mode));
347
modes[argc + j] |= ce_skip_worktree(ce) ? SPARSE : INDEX;
348
submodule_gitfile[argc + j] = NULL;
349
}
350
+
351
+ free(dst_with_slash);
352
argc += last - first;
353
goto act_on_entry;
354
}
@@ -565,6 +566,7 @@ remove_entry:
566
COMMIT_LOCK | SKIP_IF_UNCHANGED))
567
die(_("Unable to write new index file"));
568
569
+ free(dst_w_slash);
570
string_list_clear(&src_for_dst, 0);
571
string_list_clear(&dirty_paths, 0);
572
UNLEAK(source);