git_snpath(): retire and replace with strbuf_git_path()

In the previous patch, git_snpath() is modified to allocate a new strbuf buffer because vsnpath() needs that. But that makes it awkward because git_snpath() receives a pre-allocated buffer from outside and has to copy data back. Rename it to strbuf_git_path() and make it receive strbuf directly. Using git_path() in update_refs_for_switch() which used to call git_snpath() is safe because that function and all of its callers do not keep any pointer to the round-robin buffer pool allocated by get_pathname(). Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Nguyễn Thái Ngọc Duy committed Nov 30, 2014 at 15:24 UTC 1a83c240f2fe7a0caa94009f066d15fa677d32cd
5 files changed +55 -26
builtin/checkout.c
+8 -5
@@ -588,18 +588,21 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
588 if (opts->new_orphan_branch) {
589 if (opts->new_branch_log && !log_all_ref_updates) {
590 int temp;
591 - char log_file[PATH_MAX];
592 - const char *ref_name = mkpath("refs/heads/%s", opts->new_orphan_branch);
591 + struct strbuf log_file = STRBUF_INIT;
592 + int ret;
593 + const char *ref_name;
594
595 + ref_name = mkpath("refs/heads/%s", opts->new_orphan_branch);
596 temp = log_all_ref_updates;
597 log_all_ref_updates = 1;
596 - if (log_ref_setup(ref_name, log_file, sizeof(log_file))) {
598 + ret = log_ref_setup(ref_name, &log_file);
599 + log_all_ref_updates = temp;
600 + strbuf_release(&log_file);
601 + if (ret) {
602 fprintf(stderr, _("Can not do reflog for '%s'\n"),
603 opts->new_orphan_branch);
599 - log_all_ref_updates = temp;
604 return;
605 }
602 - log_all_ref_updates = temp;
606 }
607 }
608 else
cache.h
+2 -2
@@ -679,8 +679,8 @@ extern int check_repository_format(void);
679
680 extern char *mksnpath(char *buf, size_t n, const char *fmt, ...)
681 __attribute__((format (printf, 3, 4)));
682 -extern char *git_snpath(char *buf, size_t n, const char *fmt, ...)
683 - __attribute__((format (printf, 3, 4)));
682 +extern void strbuf_git_path(struct strbuf *sb, const char *fmt, ...)
683 + __attribute__((format (printf, 2, 3)));
684 extern char *git_pathdup(const char *fmt, ...)
685 __attribute__((format (printf, 1, 2)));
686 extern char *mkpathdup(const char *fmt, ...)
path.c
+2 -9
@@ -70,19 +70,12 @@ static void vsnpath(struct strbuf *buf, const char *fmt, va_list args)
70 strbuf_cleanup_path(buf);
71 }
72
73 -char *git_snpath(char *buf, size_t n, const char *fmt, ...)
73 +void strbuf_git_path(struct strbuf *sb, const char *fmt, ...)
74 {
75 - struct strbuf sb = STRBUF_INIT;
75 va_list args;
76 va_start(args, fmt);
78 - vsnpath(&sb, fmt, args);
77 + vsnpath(sb, fmt, args);
78 va_end(args);
80 - if (sb.len >= n)
81 - strlcpy(buf, bad_path, n);
82 - else
83 - memcpy(buf, sb.buf, sb.len + 1);
84 - strbuf_release(&sb);
85 - return buf;
79 }
80
81 char *git_pathdup(const char *fmt, ...)
refs.c
+42 -9
@@ -1444,7 +1444,11 @@ static int resolve_missing_loose_ref(const char *refname,
1444 }
1445
1446 /* This function needs to return a meaningful errno on failure */
1447 -const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned char *sha1, int *flags)
1447 +static const char *resolve_ref_unsafe_1(const char *refname,
1448 + int resolve_flags,
1449 + unsigned char *sha1,
1450 + int *flags,
1451 + struct strbuf *sb_path)
1452 {
1453 int depth = MAXDEPTH;
1454 ssize_t len;
@@ -1475,7 +1479,7 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
1479 bad_name = 1;
1480 }
1481 for (;;) {
1478 - char path[PATH_MAX];
1482 + const char *path;
1483 struct stat st;
1484 char *buf;
1485 int fd;
@@ -1485,7 +1489,9 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
1489 return NULL;
1490 }
1491
1488 - git_snpath(path, sizeof(path), "%s", refname);
1492 + strbuf_reset(sb_path);
1493 + strbuf_git_path(sb_path, "%s", refname);
1494 + path = sb_path->buf;
1495
1496 /*
1497 * We might have to loop back here to avoid a race
@@ -1612,6 +1618,16 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
1618 }
1619 }
1620
1621 +const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
1622 + unsigned char *sha1, int *flags)
1623 +{
1624 + struct strbuf sb_path = STRBUF_INIT;
1625 + const char *ret = resolve_ref_unsafe_1(refname, resolve_flags,
1626 + sha1, flags, &sb_path);
1627 + strbuf_release(&sb_path);
1628 + return ret;
1629 +}
1630 +
1631 char *resolve_refdup(const char *ref, int resolve_flags, unsigned char *sha1, int *flags)
1632 {
1633 const char *ret = resolve_ref_unsafe(ref, resolve_flags, sha1, flags);
@@ -2941,11 +2957,15 @@ static int copy_msg(char *buf, const char *msg)
2957 }
2958
2959 /* This function must set a meaningful errno on failure */
2944 -int log_ref_setup(const char *refname, char *logfile, int bufsize)
2960 +int log_ref_setup(const char *refname, struct strbuf *sb_logfile)
2961 {
2962 int logfd, oflags = O_APPEND | O_WRONLY;
2963 + char *logfile;
2964
2948 - git_snpath(logfile, bufsize, "logs/%s", refname);
2965 + strbuf_git_path(sb_logfile, "logs/%s", refname);
2966 + logfile = sb_logfile->buf;
2967 + /* make sure the rest of the function can't change "logfile" */
2968 + sb_logfile = NULL;
2969 if (log_all_ref_updates &&
2970 (starts_with(refname, "refs/heads/") ||
2971 starts_with(refname, "refs/remotes/") ||
@@ -2990,22 +3010,26 @@ int log_ref_setup(const char *refname, char *logfile, int bufsize)
3010 return 0;
3011 }
3012
2993 -static int log_ref_write(const char *refname, const unsigned char *old_sha1,
2994 - const unsigned char *new_sha1, const char *msg)
3013 +static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,
3014 + const unsigned char *new_sha1, const char *msg,
3015 + struct strbuf *sb_log_file)
3016 {
3017 int logfd, result, written, oflags = O_APPEND | O_WRONLY;
3018 unsigned maxlen, len;
3019 int msglen;
2999 - char log_file[PATH_MAX];
3020 + const char *log_file;
3021 char *logrec;
3022 const char *committer;
3023
3024 if (log_all_ref_updates < 0)
3025 log_all_ref_updates = !is_bare_repository();
3026
3006 - result = log_ref_setup(refname, log_file, sizeof(log_file));
3027 + result = log_ref_setup(refname, sb_log_file);
3028 if (result)
3029 return result;
3030 + log_file = sb_log_file->buf;
3031 + /* make sure the rest of the function can't change "log_file" */
3032 + sb_log_file = NULL;
3033
3034 logfd = open(log_file, oflags);
3035 if (logfd < 0)
@@ -3038,6 +3062,15 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1,
3062 return 0;
3063 }
3064
3065 +static int log_ref_write(const char *refname, const unsigned char *old_sha1,
3066 + const unsigned char *new_sha1, const char *msg)
3067 +{
3068 + struct strbuf sb = STRBUF_INIT;
3069 + int ret = log_ref_write_1(refname, old_sha1, new_sha1, msg, &sb);
3070 + strbuf_release(&sb);
3071 + return ret;
3072 +}
3073 +
3074 int is_branch(const char *refname)
3075 {
3076 return !strcmp(refname, "HEAD") || starts_with(refname, "refs/heads/");
refs.h
+1 -1
@@ -210,7 +210,7 @@ extern void unlock_ref(struct ref_lock *lock);
210 /*
211 * Setup reflog before using. Set errno to something meaningful on failure.
212 */
213 -int log_ref_setup(const char *refname, char *logfile, int bufsize);
213 +int log_ref_setup(const char *refname, struct strbuf *logfile);
214
215 /** Reads log for the value of ref during at_time. **/
216 extern int read_ref_at(const char *refname, unsigned int flags,