worktree add: let worktree_basename() return string copy worktree_basename() requires callers to do pointer arithmetic to get the actual basename. Simplify them by doing the calculations in the function and returning a copy of the basename directly. Remind programmers to free the result by renaming the function to worktree_basename_dup(). Among the three callers of the original function, two immediately make copies of the returned string before using and freeing it, which makes for an easy conversion. Convert the other one from resetting a shared strbuf to freeing the allocated string, which requires the same number of lines, but no arithmetic. The added allocation is negligible because it's small and there's only one per run of "git worktree add". Signed-off-by: René Scharfe <l.s.r@web.de> [jc: rephrased the second paragraph a bit.] Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff --git a/builtin/worktree.c b/builtin/worktree.c index e5b7d6f..e5a4196 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c
@@ -294,7 +294,7 @@ static void remove_junk_on_signal(int signo) raise(signo); } -static const char *worktree_basename(const char *path, int *olen) +static char *worktree_basename_dup(const char *path) { const char *name; int len; @@ -307,8 +307,7 @@ static const char *worktree_basename(const char *path, int *olen) while (name > path && !is_dir_sep(name[-1])) name--; - *olen = len; - return name; + return xmemdupz(name, path + len - name); } /* check that path is viable location for worktree */ @@ -462,6 +461,7 @@ static int add_worktree(const char *path, const char *refname, struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT; struct strbuf sb = STRBUF_INIT; const char *name; + char *name_to_free = NULL; struct strvec child_env = STRVEC_INIT; unsigned int counter = 0; int len, ret; @@ -489,14 +489,12 @@ static int add_worktree(const char *path, const char *refname, if (!commit && !opts->orphan) die(_("invalid reference: %s"), refname); - name = worktree_basename(path, &len); - strbuf_add(&sb, name, path + len - name); - if (!sb.len) + name = name_to_free = worktree_basename_dup(path); + if (!*name) die(_("invalid path '%s'"), path); - sanitize_refname_component(sb.buf, &sb_name); + sanitize_refname_component(name, &sb_name); if (!sb_name.len) - BUG("How come '%s' becomes empty after sanitization?", sb.buf); - strbuf_reset(&sb); + BUG("How come '%s' becomes empty after sanitization?", name); name = sb_name.buf; repo_git_path_replace(the_repository, &sb_repo, "worktrees/%s", name); len = sb_repo.len; @@ -629,6 +627,7 @@ static int add_worktree(const char *path, const char *refname, strbuf_release(&sb_git); strbuf_release(&sb_name); free_worktree(wt); + free(name_to_free); return ret; } @@ -765,10 +764,8 @@ static int dwim_orphan(const struct add_opts *opts, int opt_track, int remote) static char *dwim_branch(const char *path, char **new_branch) { - int n; int branch_exists; - const char *s = worktree_basename(path, &n); - char *branchname = xmemdupz(s, path + n - s); + char *branchname = worktree_basename_dup(path); struct strbuf ref = STRBUF_INIT; branch_exists = !check_branch_ref(&ref, branchname) && @@ -875,9 +872,7 @@ static int add(int ac, const char **av, const char *prefix, } if (opts.orphan && !new_branch) { - int n; - const char *s = worktree_basename(path, &n); - new_branch = new_branch_to_free = xmemdupz(s, path + n - s); + new_branch = new_branch_to_free = worktree_basename_dup(path); } else if (opts.orphan) { ; /* no-op */ } else if (opts.detach) {