Skip to content

Commit

Permalink
memoize common git-path "constant" files
Browse files Browse the repository at this point in the history
One of the most common uses of git_path() is to pass a
constant, like git_path("MERGE_MSG"). This has two
drawbacks:

  1. The return value is a static buffer, and the lifetime
     is dependent on other calls to git_path, etc.

  2. There's no compile-time checking of the pathname. This
     is OK for a one-off (after all, we have to spell it
     correctly at least once), but many of these constant
     strings appear throughout the code.

This patch introduces a series of functions to "memoize"
these strings, which are essentially globals for the
lifetime of the program. We compute the value once, take
ownership of the buffer, and return the cached value for
subsequent calls.  cache.h provides a helper macro for
defining these functions as one-liners, and defines a few
common ones for global use.

Using a macro is a little bit gross, but it does nicely
document the purpose of the functions. If we need to touch
them all later (e.g., because we learned how to change the
git_dir variable at runtime, and need to invalidate all of
the stored values), it will be much easier to have the
complete list.

Note that the shared-global functions have separate, manual
declarations. We could do something clever with the macros
(e.g., expand it to a declaration in some places, and a
declaration _and_ a definition in path.c). But there aren't
that many, and it's probably better to stay away from
too-magical macros.

Likewise, if we abandon the C preprocessor in favor of
generating these with a script, we could get much fancier.
E.g., normalizing "FOO/BAR-BAZ" into "git_path_foo_bar_baz".
But the small amount of saved typing is probably not worth
the resulting confusion to readers who want to grep for the
function's definition.

Signed-off-by: Jeff King <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
  • Loading branch information
peff authored and gitster committed Aug 10, 2015
1 parent 0ea68e4 commit f932729
Show file tree
Hide file tree
Showing 17 changed files with 151 additions and 119 deletions.
4 changes: 3 additions & 1 deletion attr.c
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,8 @@ static int git_attr_system(void)
return !git_env_bool("GIT_ATTR_NOSYSTEM", 0);
}

static GIT_PATH_FUNC(git_path_info_attributes, INFOATTRIBUTES_FILE)

static void bootstrap_attr_stack(void)
{
struct attr_stack *elem;
Expand Down Expand Up @@ -531,7 +533,7 @@ static void bootstrap_attr_stack(void)
debug_push(elem);
}

elem = read_attr_from_file(git_path(INFOATTRIBUTES_FILE), 1);
elem = read_attr_from_file(git_path_info_attributes(), 1);
if (!elem)
elem = xcalloc(1, sizeof(*elem));
elem->origin = NULL;
Expand Down
7 changes: 5 additions & 2 deletions bisect.c
Original file line number Diff line number Diff line change
Expand Up @@ -420,10 +420,13 @@ static int read_bisect_refs(void)
return for_each_ref_in("refs/bisect/", register_ref, NULL);
}

static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")

static void read_bisect_paths(struct argv_array *array)
{
struct strbuf str = STRBUF_INIT;
const char *filename = git_path("BISECT_NAMES");
const char *filename = git_path_bisect_names();
FILE *fp = fopen(filename, "r");

if (!fp)
Expand Down Expand Up @@ -644,7 +647,7 @@ static void exit_if_skipped_commits(struct commit_list *tried,

static int is_expected_rev(const struct object_id *oid)
{
const char *filename = git_path("BISECT_EXPECTED_REV");
const char *filename = git_path_bisect_expected_rev();
struct stat st;
struct strbuf str = STRBUF_INIT;
FILE *fp;
Expand Down
14 changes: 7 additions & 7 deletions branch.c
Original file line number Diff line number Diff line change
Expand Up @@ -302,11 +302,11 @@ void create_branch(const char *head,

void remove_branch_state(void)
{
unlink(git_path("CHERRY_PICK_HEAD"));
unlink(git_path("REVERT_HEAD"));
unlink(git_path("MERGE_HEAD"));
unlink(git_path("MERGE_RR"));
unlink(git_path("MERGE_MSG"));
unlink(git_path("MERGE_MODE"));
unlink(git_path("SQUASH_MSG"));
unlink(git_path_cherry_pick_head());
unlink(git_path_revert_head());
unlink(git_path_merge_head());
unlink(git_path_merge_rr());
unlink(git_path_merge_msg());
unlink(git_path_merge_mode());
unlink(git_path_squash_msg());
}
7 changes: 3 additions & 4 deletions builtin/blame.c
Original file line number Diff line number Diff line change
Expand Up @@ -2227,20 +2227,19 @@ static struct commit_list **append_parent(struct commit_list **tail, const unsig
static void append_merge_parents(struct commit_list **tail)
{
int merge_head;
const char *merge_head_file = git_path("MERGE_HEAD");
struct strbuf line = STRBUF_INIT;

merge_head = open(merge_head_file, O_RDONLY);
merge_head = open(git_path_merge_head(), O_RDONLY);
if (merge_head < 0) {
if (errno == ENOENT)
return;
die("cannot open '%s' for reading", merge_head_file);
die("cannot open '%s' for reading", git_path_merge_head());
}

while (!strbuf_getwholeline_fd(&line, merge_head, '\n')) {
unsigned char sha1[20];
if (line.len < 40 || get_sha1_hex(line.buf, sha1))
die("unknown line in '%s': %s", merge_head_file, line.buf);
die("unknown line in '%s': %s", git_path_merge_head(), line.buf);
tail = append_parent(tail, sha1);
}
close(merge_head);
Expand Down
32 changes: 16 additions & 16 deletions builtin/commit.c
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,9 @@ static int opt_parse_m(const struct option *opt, const char *arg, int unset)

static void determine_whence(struct wt_status *s)
{
if (file_exists(git_path("MERGE_HEAD")))
if (file_exists(git_path_merge_head()))
whence = FROM_MERGE;
else if (file_exists(git_path("CHERRY_PICK_HEAD"))) {
else if (file_exists(git_path_cherry_pick_head())) {
whence = FROM_CHERRY_PICK;
if (file_exists(git_path(SEQ_DIR)))
sequencer_in_use = 1;
Expand Down Expand Up @@ -725,12 +725,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
format_commit_message(commit, "fixup! %s\n\n",
&sb, &ctx);
hook_arg1 = "message";
} else if (!stat(git_path("MERGE_MSG"), &statbuf)) {
if (strbuf_read_file(&sb, git_path("MERGE_MSG"), 0) < 0)
} else if (!stat(git_path_merge_msg(), &statbuf)) {
if (strbuf_read_file(&sb, git_path_merge_msg(), 0) < 0)
die_errno(_("could not read MERGE_MSG"));
hook_arg1 = "merge";
} else if (!stat(git_path("SQUASH_MSG"), &statbuf)) {
if (strbuf_read_file(&sb, git_path("SQUASH_MSG"), 0) < 0)
} else if (!stat(git_path_squash_msg(), &statbuf)) {
if (strbuf_read_file(&sb, git_path_squash_msg(), 0) < 0)
die_errno(_("could not read SQUASH_MSG"));
hook_arg1 = "squash";
} else if (template_file) {
Expand Down Expand Up @@ -1684,10 +1684,10 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
if (!reflog_msg)
reflog_msg = "commit (merge)";
pptr = &commit_list_insert(current_head, pptr)->next;
fp = fopen(git_path("MERGE_HEAD"), "r");
fp = fopen(git_path_merge_head(), "r");
if (fp == NULL)
die_errno(_("could not open '%s' for reading"),
git_path("MERGE_HEAD"));
git_path_merge_head());
while (strbuf_getline(&m, fp, '\n') != EOF) {
struct commit *parent;

Expand All @@ -1698,8 +1698,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
}
fclose(fp);
strbuf_release(&m);
if (!stat(git_path("MERGE_MODE"), &statbuf)) {
if (strbuf_read_file(&sb, git_path("MERGE_MODE"), 0) < 0)
if (!stat(git_path_merge_mode(), &statbuf)) {
if (strbuf_read_file(&sb, git_path_merge_mode(), 0) < 0)
die_errno(_("could not read MERGE_MODE"));
if (!strcmp(sb.buf, "no-ff"))
allow_fast_forward = 0;
Expand Down Expand Up @@ -1775,12 +1775,12 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
}
ref_transaction_free(transaction);

unlink(git_path("CHERRY_PICK_HEAD"));
unlink(git_path("REVERT_HEAD"));
unlink(git_path("MERGE_HEAD"));
unlink(git_path("MERGE_MSG"));
unlink(git_path("MERGE_MODE"));
unlink(git_path("SQUASH_MSG"));
unlink(git_path_cherry_pick_head());
unlink(git_path_revert_head());
unlink(git_path_merge_head());
unlink(git_path_merge_msg());
unlink(git_path_merge_mode());
unlink(git_path_squash_msg());

if (commit_index_files())
die (_("Repository has been updated, but unable to write\n"
Expand Down
4 changes: 2 additions & 2 deletions builtin/fetch.c
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
const char *what, *kind;
struct ref *rm;
char *url;
const char *filename = dry_run ? "/dev/null" : git_path("FETCH_HEAD");
const char *filename = dry_run ? "/dev/null" : git_path_fetch_head();
int want_status;

fp = fopen(filename, "a");
Expand Down Expand Up @@ -834,7 +834,7 @@ static void check_not_current_branch(struct ref *ref_map)

static int truncate_fetch_head(void)
{
const char *filename = git_path("FETCH_HEAD");
const char *filename = git_path_fetch_head();
FILE *fp = fopen(filename, "w");

if (!fp)
Expand Down
30 changes: 15 additions & 15 deletions builtin/merge.c
Original file line number Diff line number Diff line change
Expand Up @@ -231,9 +231,9 @@ static struct option builtin_merge_options[] = {
/* Cleans up metadata that is uninteresting after a succeeded merge. */
static void drop_save(void)
{
unlink(git_path("MERGE_HEAD"));
unlink(git_path("MERGE_MSG"));
unlink(git_path("MERGE_MODE"));
unlink(git_path_merge_head());
unlink(git_path_merge_msg());
unlink(git_path_merge_mode());
}

static int save_state(unsigned char *stash)
Expand Down Expand Up @@ -338,7 +338,7 @@ static void squash_message(struct commit *commit, struct commit_list *remotehead
struct pretty_print_context ctx = {0};

printf(_("Squash commit -- not updating HEAD\n"));
filename = git_path("SQUASH_MSG");
filename = git_path_squash_msg();
fd = open(filename, O_WRONLY | O_CREAT, 0666);
if (fd < 0)
die_errno(_("Could not write to '%s'"), filename);
Expand Down Expand Up @@ -754,7 +754,7 @@ static void add_strategies(const char *string, unsigned attr)

static void write_merge_msg(struct strbuf *msg)
{
const char *filename = git_path("MERGE_MSG");
const char *filename = git_path_merge_msg();
int fd = open(filename, O_WRONLY | O_CREAT, 0666);
if (fd < 0)
die_errno(_("Could not open '%s' for writing"),
Expand All @@ -766,7 +766,7 @@ static void write_merge_msg(struct strbuf *msg)

static void read_merge_msg(struct strbuf *msg)
{
const char *filename = git_path("MERGE_MSG");
const char *filename = git_path_merge_msg();
strbuf_reset(msg);
if (strbuf_read_file(msg, filename, 0) < 0)
die_errno(_("Could not read from '%s'"), filename);
Expand Down Expand Up @@ -799,10 +799,10 @@ static void prepare_to_commit(struct commit_list *remoteheads)
strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);
write_merge_msg(&msg);
if (run_commit_hook(0 < option_edit, get_index_file(), "prepare-commit-msg",
git_path("MERGE_MSG"), "merge", NULL))
git_path_merge_msg(), "merge", NULL))
abort_commit(remoteheads, NULL);
if (0 < option_edit) {
if (launch_editor(git_path("MERGE_MSG"), NULL, NULL))
if (launch_editor(git_path_merge_msg(), NULL, NULL))
abort_commit(remoteheads, NULL);
}
read_merge_msg(&msg);
Expand Down Expand Up @@ -865,7 +865,7 @@ static int suggest_conflicts(void)
FILE *fp;
struct strbuf msgbuf = STRBUF_INIT;

filename = git_path("MERGE_MSG");
filename = git_path_merge_msg();
fp = fopen(filename, "a");
if (!fp)
die_errno(_("Could not open '%s' for writing"), filename);
Expand Down Expand Up @@ -967,7 +967,7 @@ static void write_merge_state(struct commit_list *remoteheads)
}
strbuf_addf(&buf, "%s\n", sha1_to_hex(sha1));
}
filename = git_path("MERGE_HEAD");
filename = git_path_merge_head();
fd = open(filename, O_WRONLY | O_CREAT, 0666);
if (fd < 0)
die_errno(_("Could not open '%s' for writing"), filename);
Expand All @@ -977,7 +977,7 @@ static void write_merge_state(struct commit_list *remoteheads)
strbuf_addch(&merge_msg, '\n');
write_merge_msg(&merge_msg);

filename = git_path("MERGE_MODE");
filename = git_path_merge_mode();
fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, 0666);
if (fd < 0)
die_errno(_("Could not open '%s' for writing"), filename);
Expand Down Expand Up @@ -1070,7 +1070,7 @@ static void handle_fetch_head(struct commit_list **remotes, struct strbuf *merge
if (!merge_names)
merge_names = &fetch_head_file;

filename = git_path("FETCH_HEAD");
filename = git_path_fetch_head();
fd = open(filename, O_RDONLY);
if (fd < 0)
die_errno(_("could not open '%s' for reading"), filename);
Expand Down Expand Up @@ -1204,7 +1204,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
int nargc = 2;
const char *nargv[] = {"reset", "--merge", NULL};

if (!file_exists(git_path("MERGE_HEAD")))
if (!file_exists(git_path_merge_head()))
die(_("There is no merge to abort (MERGE_HEAD missing)."));

/* Invoke 'git reset --merge' */
Expand All @@ -1215,7 +1215,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
if (read_cache_unmerged())
die_resolve_conflict("merge");

if (file_exists(git_path("MERGE_HEAD"))) {
if (file_exists(git_path_merge_head())) {
/*
* There is no unmerged entry, don't advise 'git
* add/rm <file>', just 'git commit'.
Expand All @@ -1226,7 +1226,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
else
die(_("You have not concluded your merge (MERGE_HEAD exists)."));
}
if (file_exists(git_path("CHERRY_PICK_HEAD"))) {
if (file_exists(git_path_cherry_pick_head())) {
if (advice_resolve_conflict)
die(_("You have not concluded your cherry-pick (CHERRY_PICK_HEAD exists).\n"
"Please, commit your changes before you merge."));
Expand Down
2 changes: 1 addition & 1 deletion builtin/reset.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ static const char *reset_type_names[] = {

static inline int is_merge(void)
{
return !access(git_path("MERGE_HEAD"), F_OK);
return !access(git_path_merge_head(), F_OK);
}

static int reset_index(const unsigned char *sha1, int reset_type, int quiet)
Expand Down
26 changes: 26 additions & 0 deletions cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,32 @@ extern char *git_pathdup_submodule(const char *path, const char *fmt, ...)

extern void report_linked_checkout_garbage(void);

/*
* You can define a static memoized git path like:
*
* static GIT_PATH_FUNC(git_path_foo, "FOO");
*
* or use one of the global ones below.
*/
#define GIT_PATH_FUNC(func, filename) \
const char *func(void) \
{ \
static char *ret; \
if (!ret) \
ret = git_pathdup(filename); \
return ret; \
}

const char *git_path_cherry_pick_head(void);
const char *git_path_revert_head(void);
const char *git_path_squash_msg(void);
const char *git_path_merge_msg(void);
const char *git_path_merge_rr(void);
const char *git_path_merge_mode(void);
const char *git_path_merge_head(void);
const char *git_path_fetch_head(void);
const char *git_path_shallow(void);

/*
* Return the name of the file in the local object database that would
* be used to store a loose object with the specified sha1. The
Expand Down
4 changes: 2 additions & 2 deletions contrib/examples/builtin-fetch--tool.c
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ int cmd_fetch__tool(int argc, const char **argv, const char *prefix)

if (argc != 8)
return error("append-fetch-head takes 6 args");
filename = git_path("FETCH_HEAD");
filename = git_path_fetch_head();
fp = fopen(filename, "a");
if (!fp)
return error("cannot open %s: %s", filename, strerror(errno));
Expand All @@ -534,7 +534,7 @@ int cmd_fetch__tool(int argc, const char **argv, const char *prefix)

if (argc != 5)
return error("fetch-native-store takes 3 args");
filename = git_path("FETCH_HEAD");
filename = git_path_fetch_head();
fp = fopen(filename, "a");
if (!fp)
return error("cannot open %s: %s", filename, strerror(errno));
Expand Down
4 changes: 3 additions & 1 deletion dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -2185,6 +2185,8 @@ int remove_dir_recursively(struct strbuf *path, int flag)
return remove_dir_recurse(path, flag, NULL);
}

static GIT_PATH_FUNC(git_path_info_exclude, "info/exclude")

void setup_standard_excludes(struct dir_struct *dir)
{
const char *path;
Expand All @@ -2199,7 +2201,7 @@ void setup_standard_excludes(struct dir_struct *dir)
dir->untracked ? &dir->ss_excludes_file : NULL);

/* per repository user preference */
path = git_path("info/exclude");
path = git_path_info_exclude();
if (!access_or_warn(path, R_OK, 0))
add_excludes_from_file_1(dir, path,
dir->untracked ? &dir->ss_info_exclude : NULL);
Expand Down
2 changes: 1 addition & 1 deletion fetch-pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -948,7 +948,7 @@ static void update_shallow(struct fetch_pack_args *args,

if (args->depth > 0 && alternate_shallow_file) {
if (*alternate_shallow_file == '\0') { /* --unshallow */
unlink_or_warn(git_path("shallow"));
unlink_or_warn(git_path_shallow());
rollback_lock_file(&shallow_lock);
} else
commit_lock_file(&shallow_lock);
Expand Down
10 changes: 10 additions & 0 deletions path.c
Original file line number Diff line number Diff line change
Expand Up @@ -933,3 +933,13 @@ char *xdg_config_home(const char *filename)
return mkpathdup("%s/.config/git/%s", home, filename);
return NULL;
}

GIT_PATH_FUNC(git_path_cherry_pick_head, "CHERRY_PICK_HEAD")
GIT_PATH_FUNC(git_path_revert_head, "REVERT_HEAD")
GIT_PATH_FUNC(git_path_squash_msg, "SQUASH_MSG")
GIT_PATH_FUNC(git_path_merge_msg, "MERGE_MSG")
GIT_PATH_FUNC(git_path_merge_rr, "MERGE_RR")
GIT_PATH_FUNC(git_path_merge_mode, "MERGE_MODE")
GIT_PATH_FUNC(git_path_merge_head, "MERGE_HEAD")
GIT_PATH_FUNC(git_path_fetch_head, "FETCH_HEAD")
GIT_PATH_FUNC(git_path_shallow, "shallow")
Loading

0 comments on commit f932729

Please sign in to comment.