From f5d284f6df5413f1fcdf2c92bbf0e8523298c483 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Apr 2017 17:08:25 -0400 Subject: bisect: add git_path_bisect_terms helper This avoids using the dangerous git_path(). Right now there's only one call site (because the writing half is still part of the shell script), but it may come in handy in the future as more of bisect is written in C. It also matches how we access the other BISECT_* files. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- bisect.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bisect.c b/bisect.c index 30808cadf..fb9c9b126 100644 --- a/bisect.c +++ b/bisect.c @@ -430,6 +430,7 @@ static int read_bisect_refs(void) static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES") static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV") +static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS") static void read_bisect_paths(struct argv_array *array) { @@ -905,7 +906,7 @@ static void show_diff_tree(const char *prefix, struct commit *commit) void read_bisect_terms(const char **read_bad, const char **read_good) { struct strbuf str = STRBUF_INIT; - const char *filename = git_path("BISECT_TERMS"); + const char *filename = git_path_bisect_terms(); FILE *fp = fopen(filename, "r"); if (!fp) { -- cgit v1.2.1 From c10388c7dc89ec4f8a6c1f7b1fd15d70e8ee0f07 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Apr 2017 17:08:41 -0400 Subject: branch: add edit_description() helper Rather than have a variable with a short name that is fed to git_path(), let's add a helper function that returns the full path. This avoids the dangerous git_path() function. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/branch.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index babfd0f73..c55ea5804 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -600,7 +600,7 @@ static void rename_branch(const char *oldname, const char *newname, int force) strbuf_release(&newsection); } -static const char edit_description[] = "BRANCH_DESCRIPTION"; +static GIT_PATH_FUNC(edit_description, "EDIT_DESCRIPTION") static int edit_branch_description(const char *branch_name) { @@ -615,9 +615,9 @@ static int edit_branch_description(const char *branch_name) " %s\n" "Lines starting with '%c' will be stripped.\n"), branch_name, comment_line_char); - write_file_buf(git_path(edit_description), buf.buf, buf.len); + write_file_buf(edit_description(), buf.buf, buf.len); strbuf_reset(&buf); - if (launch_editor(git_path(edit_description), &buf, NULL)) { + if (launch_editor(edit_description(), &buf, NULL)) { strbuf_release(&buf); return -1; } -- cgit v1.2.1 From ca03e0670c3ba71400b0f5ed199b045481efdb1e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Apr 2017 17:08:54 -0400 Subject: use git_path_* helper functions Long ago we added functions like git_path_merge_msg() to replace the more dangerous git_path("MERGE_MSG"). Over time some new calls to the latter have crept it. Let's convert them to use the safer form. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/commit.c | 6 +++--- builtin/pull.c | 4 ++-- sequencer.c | 12 ++++++------ 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 2de5f6cc6..bbaa4a0da 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -821,9 +821,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix, "If this is not correct, please remove the file\n" " %s\n" "and try again.\n"), - git_path(whence == FROM_MERGE - ? "MERGE_HEAD" - : "CHERRY_PICK_HEAD")); + whence == FROM_MERGE ? + git_path_merge_head() : + git_path_cherry_pick_head()); } fprintf(s->fp, "\n"); diff --git a/builtin/pull.c b/builtin/pull.c index 3ecb881b0..d8032e838 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -332,7 +332,7 @@ static int git_pull_config(const char *var, const char *value, void *cb) */ static void get_merge_heads(struct sha1_array *merge_heads) { - const char *filename = git_path("FETCH_HEAD"); + const char *filename = git_path_fetch_head(); FILE *fp; struct strbuf sb = STRBUF_INIT; unsigned char sha1[GIT_SHA1_RAWSZ]; @@ -791,7 +791,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix) if (read_cache_unmerged()) die_resolve_conflict("pull"); - if (file_exists(git_path("MERGE_HEAD"))) + if (file_exists(git_path_merge_head())) die_conclude_merge(); if (get_sha1("HEAD", orig_head)) diff --git a/sequencer.c b/sequencer.c index 1f729b053..f52c30fbe 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1057,12 +1057,12 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, cleanup_commit_message = 1; msg_file = rebase_path_fixup_msg(); } else { - const char *dest = git_path("SQUASH_MSG"); + const char *dest = git_path_squash_msg(); unlink(dest); if (copy_file(dest, rebase_path_squash_msg(), 0666)) return error(_("could not rename '%s' to '%s'"), rebase_path_squash_msg(), dest); - unlink(git_path("MERGE_MSG")); + unlink(git_path_merge_msg()); msg_file = dest; edit = 1; } @@ -1812,10 +1812,10 @@ static int error_failed_squash(struct commit *commit, return error(_("could not rename '%s' to '%s'"), rebase_path_squash_msg(), rebase_path_message()); unlink(rebase_path_fixup_msg()); - unlink(git_path("MERGE_MSG")); - if (copy_file(git_path("MERGE_MSG"), rebase_path_message(), 0666)) + unlink(git_path_merge_msg()); + if (copy_file(git_path_merge_msg(), rebase_path_message(), 0666)) return error(_("could not copy '%s' to '%s'"), - rebase_path_message(), git_path("MERGE_MSG")); + rebase_path_message(), git_path_merge_msg()); return error_with_patch(commit, subject, subject_len, opts, 1, 0); } @@ -2158,7 +2158,7 @@ static int commit_staged_changes(struct replay_opts *opts) if (has_unstaged_changes(1)) return error(_("cannot rebase: You have unstaged changes.")); if (!has_uncommitted_changes(0)) { - const char *cherry_pick_head = git_path("CHERRY_PICK_HEAD"); + const char *cherry_pick_head = git_path_cherry_pick_head(); if (file_exists(cherry_pick_head) && unlink(cherry_pick_head)) return error(_("could not remove CHERRY_PICK_HEAD")); -- cgit v1.2.1 From d9c69644b27eb59fe16c1931580e6fce4abbdc65 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Apr 2017 17:09:09 -0400 Subject: replace xstrdup(git_path(...)) with git_pathdup(...) It's more efficient to use git_pathdup(), as it skips an extra copy of the path. And by removing some calls to git_path(), it makes it easier to audit for dangerous uses. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/config.c | 5 +++-- fast-import.c | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/builtin/config.c b/builtin/config.c index 05843a0f9..07fcc2388 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -599,8 +599,9 @@ int cmd_config(int argc, const char **argv, const char *prefix) if (given_config_source.blob) die("editing blobs is not supported"); git_config(git_default_config, NULL); - config_file = xstrdup(given_config_source.file ? - given_config_source.file : git_path("config")); + config_file = given_config_source.file ? + xstrdup(given_config_source.file) : + git_pathdup("config"); if (use_global_config) { int fd = open(config_file, O_CREAT | O_EXCL | O_WRONLY, 0666); if (fd >= 0) { diff --git a/fast-import.c b/fast-import.c index 4d5a7f58d..d2ef8a872 100644 --- a/fast-import.c +++ b/fast-import.c @@ -3203,7 +3203,7 @@ static char* make_fast_import_path(const char *path) { if (!relative_marks_paths || is_absolute_path(path)) return xstrdup(path); - return xstrdup(git_path("info/fast-import/%s", path)); + return git_pathdup("info/fast-import/%s", path); } static void option_import_marks(const char *marks, -- cgit v1.2.1 From 8c2ca3a6d6d0bf51332f92d25b7902f9943aaaf2 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Apr 2017 17:09:30 -0400 Subject: replace strbuf_addstr(git_path()) with git_path_buf() Writing directly into the strbuf avoids a useless copy of the data, and dropping calls to git_path() makes it easier to audit for dangerous calls. Note that git_path() does an implicit strbuf_reset(), but in each of these cases we were either already doing that reset, or writing into a fresh strbuf anyway. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/worktree.c | 6 ++---- notes-merge.c | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index 831fe058a..9cceaf451 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -106,8 +106,7 @@ static void prune_worktrees(void) printf("%s\n", reason.buf); if (show_only) continue; - strbuf_reset(&path); - strbuf_addstr(&path, git_path("worktrees/%s", d->d_name)); + git_path_buf(&path, "worktrees/%s", d->d_name); ret = remove_dir_recursively(&path, 0); if (ret < 0 && errno == ENOTDIR) ret = unlink(path.buf); @@ -215,8 +214,7 @@ static int add_worktree(const char *path, const char *refname, } name = worktree_basename(path, &len); - strbuf_addstr(&sb_repo, - git_path("worktrees/%.*s", (int)(path + len - name), name)); + git_path_buf(&sb_repo, "worktrees/%.*s", (int)(path + len - name), name); len = sb_repo.len; if (safe_create_leading_directories_const(sb_repo.buf)) die_errno(_("could not create leading directories of '%s'"), diff --git a/notes-merge.c b/notes-merge.c index 5998605ac..32caaaff7 100644 --- a/notes-merge.c +++ b/notes-merge.c @@ -676,7 +676,7 @@ int notes_merge_commit(struct notes_merge_options *o, const char *msg = strstr(buffer, "\n\n"); int baselen; - strbuf_addstr(&path, git_path(NOTES_MERGE_WORKTREE)); + git_path_buf(&path, NOTES_MERGE_WORKTREE); if (o->verbosity >= 3) printf("Committing notes in notes merge worktree at %s\n", path.buf); @@ -741,7 +741,7 @@ int notes_merge_abort(struct notes_merge_options *o) struct strbuf buf = STRBUF_INIT; int ret; - strbuf_addstr(&buf, git_path(NOTES_MERGE_WORKTREE)); + git_path_buf(&buf, NOTES_MERGE_WORKTREE); if (o->verbosity >= 3) printf("Removing notes merge worktree at %s/*\n", buf.buf); ret = remove_dir_recursively(&buf, REMOVE_DIR_KEEP_TOPLEVEL); -- cgit v1.2.1 From 16d2676c9ee996208277772fdf81dda212355440 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Apr 2017 17:09:35 -0400 Subject: am: drop "dir" parameter from am_state_init The only caller of this function passes in a static buffer returned from git_path(). This looks dangerous at first glance, but turns out to be OK because the first thing we do is xstrdup() the result. Let's turn this into a git_pathdup(). That's slightly more efficient (no extra copy), and makes it easier to audit for dangerous git_path() invocations. Since there's only a single caller, let's just set this default path inside the init function. That makes the memory ownership clear. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/am.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 31fb60578..7a6cf9533 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -134,17 +134,15 @@ struct am_state { }; /** - * Initializes am_state with the default values. The state directory is set to - * dir. + * Initializes am_state with the default values. */ -static void am_state_init(struct am_state *state, const char *dir) +static void am_state_init(struct am_state *state) { int gpgsign; memset(state, 0, sizeof(*state)); - assert(dir); - state->dir = xstrdup(dir); + state->dir = git_pathdup("rebase-apply"); state->prec = 4; @@ -2322,7 +2320,7 @@ int cmd_am(int argc, const char **argv, const char *prefix) git_config(git_am_config, NULL); - am_state_init(&state, git_path("rebase-apply")); + am_state_init(&state); in_progress = am_in_progress(&state); if (in_progress) -- cgit v1.2.1