From 108bebeab31881654b7b0f1b5b393a6655d74d3f Mon Sep 17 00:00:00 2001 From: Alex Riesen Date: Sun, 26 Oct 2008 22:59:13 +0100 Subject: [PATCH 1/7] Add mksnpath which allows you to specify the output buffer This is just vsnprintf's but additionally calls cleanup_path() on the result. To be used as alternatives to mkpath() where the buffer for the created path may not be reused by subsequent calls of the same formatting function. Signed-off-by: Alex Riesen Signed-off-by: Junio C Hamano --- cache.h | 3 +++ path.c | 15 +++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/cache.h b/cache.h index 884fae826c..aea13b0822 100644 --- a/cache.h +++ b/cache.h @@ -480,6 +480,9 @@ extern int check_repository_format(void); #define DATA_CHANGED 0x0020 #define TYPE_CHANGED 0x0040 +extern char *mksnpath(char *buf, size_t n, const char *fmt, ...) + __attribute__((format (printf, 3, 4))); + /* Return a statically allocated filename matching the sha1 signature */ extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2))); extern char *git_path(const char *fmt, ...) __attribute__((format (printf, 1, 2))); diff --git a/path.c b/path.c index 76e8872622..8b64878c21 100644 --- a/path.c +++ b/path.c @@ -32,6 +32,21 @@ static char *cleanup_path(char *path) return path; } +char *mksnpath(char *buf, size_t n, const char *fmt, ...) +{ + va_list args; + unsigned len; + + va_start(args, fmt); + len = vsnprintf(buf, n, fmt, args); + va_end(args); + if (len >= n) { + snprintf(buf, n, bad_path); + return buf; + } + return cleanup_path(buf); +} + char *mkpath(const char *fmt, ...) { va_list args; From 94cc355287a7efc3eda76af6ae31f503a1ac098b Mon Sep 17 00:00:00 2001 From: Alex Riesen Date: Sun, 26 Oct 2008 23:07:24 +0100 Subject: [PATCH 2/7] Fix mkpath abuse in dwim_ref and dwim_log of sha1_name.c Otherwise the function sometimes fail to resolve obviously correct refnames, because the string data pointed to by "str" argument were reused. The change in dwim_log does not fix anything, just optimizes away strcpy code as the path can be created directly in the available buffer. Signed-off-by: Alex Riesen Signed-off-by: Junio C Hamano --- sha1_name.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 4fb77f8863..75a5a7e96f 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -245,11 +245,13 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref) *ref = NULL; for (p = ref_rev_parse_rules; *p; p++) { + char fullref[PATH_MAX]; unsigned char sha1_from_ref[20]; unsigned char *this_result; this_result = refs_found ? sha1_from_ref : sha1; - r = resolve_ref(mkpath(*p, len, str), this_result, 1, NULL); + mksnpath(fullref, sizeof(fullref), *p, len, str); + r = resolve_ref(fullref, this_result, 1, NULL); if (r) { if (!refs_found++) *ref = xstrdup(r); @@ -272,7 +274,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log) char path[PATH_MAX]; const char *ref, *it; - strcpy(path, mkpath(*p, len, str)); + mksnpath(path, sizeof(path), *p, len, str); ref = resolve_ref(path, hash, 1, NULL); if (!ref) continue; From 9fa03c177ff826b439537072338af958fe01c257 Mon Sep 17 00:00:00 2001 From: Alex Riesen Date: Sun, 26 Oct 2008 23:08:52 +0100 Subject: [PATCH 3/7] Fix potentially dangerous uses of mkpath and git_path Replace them with mksnpath/git_snpath and a local buffer for the resulting string. Signed-off-by: Alex Riesen Signed-off-by: Junio C Hamano --- builtin-apply.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin-apply.c b/builtin-apply.c index e9d49f133a..50b623e54c 100644 --- a/builtin-apply.c +++ b/builtin-apply.c @@ -2850,8 +2850,8 @@ static void create_one_file(char *path, unsigned mode, const char *buf, unsigned unsigned int nr = getpid(); for (;;) { - const char *newpath; - newpath = mkpath("%s~%u", path, nr); + char newpath[PATH_MAX]; + mksnpath(newpath, sizeof(newpath), "%s~%u", path, nr); if (!try_create_file(newpath, mode, buf, size)) { if (!rename(newpath, path)) return; From fe2d7776d5191896e361973f478ca078fa95b324 Mon Sep 17 00:00:00 2001 From: Alex Riesen Date: Mon, 27 Oct 2008 10:22:21 +0100 Subject: [PATCH 4/7] Add git_snpath: a .git path formatting routine with output buffer The function's purpose is to replace git_path where the buffer of formatted path may not be reused by subsequent calls of the function or will be copied anyway. Signed-off-by: Junio C Hamano --- cache.h | 2 ++ path.c | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/cache.h b/cache.h index aea13b0822..3d5a08ea81 100644 --- a/cache.h +++ b/cache.h @@ -482,6 +482,8 @@ extern int check_repository_format(void); extern char *mksnpath(char *buf, size_t n, const char *fmt, ...) __attribute__((format (printf, 3, 4))); +extern char *git_snpath(char *buf, size_t n, const char *fmt, ...) + __attribute__((format (printf, 3, 4))); /* Return a statically allocated filename matching the sha1 signature */ extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2))); diff --git a/path.c b/path.c index 8b64878c21..85ab28a0f1 100644 --- a/path.c +++ b/path.c @@ -47,6 +47,29 @@ char *mksnpath(char *buf, size_t n, const char *fmt, ...) return cleanup_path(buf); } +char *git_snpath(char *buf, size_t n, const char *fmt, ...) +{ + const char *git_dir = get_git_dir(); + va_list args; + size_t len; + + len = strlen(git_dir); + if (n < len + 1) + goto bad; + memcpy(buf, git_dir, len); + if (len && !is_dir_sep(git_dir[len-1])) + buf[len++] = '/'; + va_start(args, fmt); + len += vsnprintf(buf + len, n - len, fmt, args); + va_end(args); + if (len >= n) + goto bad; + return cleanup_path(buf); +bad: + snprintf(buf, n, bad_path); + return buf; +} + char *mkpath(const char *fmt, ...) { va_list args; From 958a4789e0e74da245175e31bd3b9b354ee0e063 Mon Sep 17 00:00:00 2001 From: Alex Riesen Date: Mon, 27 Oct 2008 11:11:40 +0100 Subject: [PATCH 5/7] Fix potentially dangerous use of git_path in ref.c Signed-off-by: Junio C Hamano --- refs.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/refs.c b/refs.c index 39a3b23804..71443cdf85 100644 --- a/refs.c +++ b/refs.c @@ -401,7 +401,7 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int * *flag = 0; for (;;) { - const char *path = git_path("%s", ref); + char path[PATH_MAX]; struct stat st; char *buf; int fd; @@ -409,6 +409,7 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int * if (--depth < 0) return NULL; + git_snpath(path, sizeof(path), "%s", ref); /* Special case: non-existing file. * Not having the refs/heads/new-branch is OK * if we are writing into it, so is .git/HEAD @@ -1121,13 +1122,14 @@ static int log_ref_write(const char *ref_name, const unsigned char *old_sha1, int logfd, written, oflags = O_APPEND | O_WRONLY; unsigned maxlen, len; int msglen; - char *log_file, *logrec; + char log_file[PATH_MAX]; + char *logrec; const char *committer; if (log_all_ref_updates < 0) log_all_ref_updates = !is_bare_repository(); - log_file = git_path("logs/%s", ref_name); + git_snpath(log_file, sizeof(log_file), "logs/%s", ref_name); if (log_all_ref_updates && (!prefixcmp(ref_name, "refs/heads/") || From aba13e7c0566f578f866504bfcb388a72f7e5079 Mon Sep 17 00:00:00 2001 From: Alex Riesen Date: Mon, 27 Oct 2008 11:17:51 +0100 Subject: [PATCH 6/7] git_pathdup: returns xstrdup-ed copy of the formatted path Signed-off-by: Junio C Hamano --- cache.h | 2 ++ path.c | 24 ++++++++++++++++++++---- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/cache.h b/cache.h index 3d5a08ea81..eaacd6dd9f 100644 --- a/cache.h +++ b/cache.h @@ -484,6 +484,8 @@ extern char *mksnpath(char *buf, size_t n, const char *fmt, ...) __attribute__((format (printf, 3, 4))); extern char *git_snpath(char *buf, size_t n, const char *fmt, ...) __attribute__((format (printf, 3, 4))); +extern char *git_pathdup(const char *fmt, ...) + __attribute__((format (printf, 1, 2))); /* Return a statically allocated filename matching the sha1 signature */ extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2))); diff --git a/path.c b/path.c index 85ab28a0f1..092ce57190 100644 --- a/path.c +++ b/path.c @@ -47,10 +47,9 @@ char *mksnpath(char *buf, size_t n, const char *fmt, ...) return cleanup_path(buf); } -char *git_snpath(char *buf, size_t n, const char *fmt, ...) +static char *git_vsnpath(char *buf, size_t n, const char *fmt, va_list args) { const char *git_dir = get_git_dir(); - va_list args; size_t len; len = strlen(git_dir); @@ -59,9 +58,7 @@ char *git_snpath(char *buf, size_t n, const char *fmt, ...) memcpy(buf, git_dir, len); if (len && !is_dir_sep(git_dir[len-1])) buf[len++] = '/'; - va_start(args, fmt); len += vsnprintf(buf + len, n - len, fmt, args); - va_end(args); if (len >= n) goto bad; return cleanup_path(buf); @@ -70,6 +67,25 @@ char *git_snpath(char *buf, size_t n, const char *fmt, ...) return buf; } +char *git_snpath(char *buf, size_t n, const char *fmt, ...) +{ + va_list args; + va_start(args, fmt); + (void)git_vsnpath(buf, n, fmt, args); + va_end(args); + return buf; +} + +char *git_pathdup(const char *fmt, ...) +{ + char path[PATH_MAX]; + va_list args; + va_start(args, fmt); + (void)git_vsnpath(path, sizeof(path), fmt, args); + va_end(args); + return xstrdup(path); +} + char *mkpath(const char *fmt, ...) { va_list args; From a4f34cbb4cea1f0b0e625b528f269f4b517c64f8 Mon Sep 17 00:00:00 2001 From: Alex Riesen Date: Mon, 27 Oct 2008 11:22:09 +0100 Subject: [PATCH 7/7] Use git_pathdup instead of xstrdup(git_path(...)) Signed-off-by: Junio C Hamano --- builtin-config.c | 2 +- builtin-reflog.c | 4 ++-- builtin-revert.c | 2 +- builtin-tag.c | 2 +- config.c | 6 +++--- environment.c | 2 +- refs.c | 2 +- rerere.c | 2 +- server-info.c | 2 +- 9 files changed, 12 insertions(+), 12 deletions(-) diff --git a/builtin-config.c b/builtin-config.c index 91fdc4985d..f71016204b 100644 --- a/builtin-config.c +++ b/builtin-config.c @@ -84,7 +84,7 @@ static int get_value(const char* key_, const char* regex_) local = config_exclusive_filename; if (!local) { const char *home = getenv("HOME"); - local = repo_config = xstrdup(git_path("config")); + local = repo_config = git_pathdup("config"); if (git_config_global() && home) global = xstrdup(mkpath("%s/.gitconfig", home)); if (git_config_system()) diff --git a/builtin-reflog.c b/builtin-reflog.c index 196fa03b7f..da96da317b 100644 --- a/builtin-reflog.c +++ b/builtin-reflog.c @@ -277,11 +277,11 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused, lock = lock_any_ref_for_update(ref, sha1, 0); if (!lock) return error("cannot lock ref '%s'", ref); - log_file = xstrdup(git_path("logs/%s", ref)); + log_file = git_pathdup("logs/%s", ref); if (!file_exists(log_file)) goto finish; if (!cmd->dry_run) { - newlog_path = xstrdup(git_path("logs/%s.lock", ref)); + newlog_path = git_pathdup("logs/%s.lock", ref); cb.newlog = fopen(newlog_path, "w"); } diff --git a/builtin-revert.c b/builtin-revert.c index 27881e9493..5c4ab58f46 100644 --- a/builtin-revert.c +++ b/builtin-revert.c @@ -269,7 +269,7 @@ static int revert_or_cherry_pick(int argc, const char **argv) int i; char *oneline, *reencoded_message = NULL; const char *message, *encoding; - const char *defmsg = xstrdup(git_path("MERGE_MSG")); + const char *defmsg = git_pathdup("MERGE_MSG"); git_config(git_default_config, NULL); me = action == REVERT ? "revert" : "cherry-pick"; diff --git a/builtin-tag.c b/builtin-tag.c index f2853d08c7..6c6c35176e 100644 --- a/builtin-tag.c +++ b/builtin-tag.c @@ -283,7 +283,7 @@ static void create_tag(const unsigned char *object, const char *tag, int fd; /* write the template message before editing: */ - path = xstrdup(git_path("TAG_EDITMSG")); + path = git_pathdup("TAG_EDITMSG"); fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0600); if (fd < 0) die("could not create file '%s': %s", diff --git a/config.c b/config.c index 53f04a076a..82807c83b2 100644 --- a/config.c +++ b/config.c @@ -630,7 +630,7 @@ int git_config(config_fn_t fn, void *data) free(user_config); } - repo_config = xstrdup(git_path("config")); + repo_config = git_pathdup("config"); ret += git_config_from_file(fn, repo_config, data); free(repo_config); return ret; @@ -872,7 +872,7 @@ int git_config_set_multivar(const char* key, const char* value, if (config_exclusive_filename) config_filename = xstrdup(config_exclusive_filename); else - config_filename = xstrdup(git_path("config")); + config_filename = git_pathdup("config"); /* * Since "key" actually contains the section name and the real @@ -1132,7 +1132,7 @@ int git_config_rename_section(const char *old_name, const char *new_name) if (config_exclusive_filename) config_filename = xstrdup(config_exclusive_filename); else - config_filename = xstrdup(git_path("config")); + config_filename = git_pathdup("config"); out_fd = hold_lock_file_for_update(lock, config_filename, 0); if (out_fd < 0) { ret = error("could not lock config file %s", config_filename); diff --git a/environment.c b/environment.c index 0c6d11f6a0..df4f03a95f 100644 --- a/environment.c +++ b/environment.c @@ -71,7 +71,7 @@ static void setup_git_env(void) } git_graft_file = getenv(GRAFT_ENVIRONMENT); if (!git_graft_file) - git_graft_file = xstrdup(git_path("info/grafts")); + git_graft_file = git_pathdup("info/grafts"); } int is_bare_repository(void) diff --git a/refs.c b/refs.c index 71443cdf85..d589b25562 100644 --- a/refs.c +++ b/refs.c @@ -1258,7 +1258,7 @@ int create_symref(const char *ref_target, const char *refs_heads_master, const char *lockpath; char ref[1000]; int fd, len, written; - char *git_HEAD = xstrdup(git_path("%s", ref_target)); + char *git_HEAD = git_pathdup("%s", ref_target); unsigned char old_sha1[20], new_sha1[20]; if (logmsg && read_ref(ref_target, old_sha1)) diff --git a/rerere.c b/rerere.c index 323e493daf..3d6ee8fa2a 100644 --- a/rerere.c +++ b/rerere.c @@ -345,7 +345,7 @@ int setup_rerere(struct string_list *merge_rr) if (!is_rerere_enabled()) return -1; - merge_rr_path = xstrdup(git_path("MERGE_RR")); + merge_rr_path = git_pathdup("MERGE_RR"); fd = hold_lock_file_for_update(&write_lock, merge_rr_path, 1); read_rr(merge_rr); return fd; diff --git a/server-info.c b/server-info.c index c1c073b2f0..66b0d9d878 100644 --- a/server-info.c +++ b/server-info.c @@ -25,7 +25,7 @@ static int add_info_ref(const char *path, const unsigned char *sha1, int flag, v static int update_info_refs(int force) { - char *path0 = xstrdup(git_path("info/refs")); + char *path0 = git_pathdup("info/refs"); int len = strlen(path0); char *path1 = xmalloc(len + 2);