[PATCH/RFC] introduce strbuf_addpath()
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Nov 16, 2011, 21:50 UTC
- Message-ID
- <20111116215004.GA29872@elie.hsd1.il.comcast.net>
- In-Reply-To
- <20111116085944.GA18781@elie.hsd1.il.comcast.net>
strbuf_addpath() is like git_path_unsafe(), except instead of returning its own buffer, it appends its result to a buffer provided by the caller.
Benefits:
- Since it uses a caller-supplied buffer, unlike git_path_unsafe(), there is no risk that one call will clobber the result from another.
- Unlike git_pathdup(), it does not need to waste time allocating memory in the middle of your tight loop over refs.
- The size of the result is not limited to PATH_MAX.
Caveat: the size of its result is not limited to PATH_MAX. Existing code might be relying on git_path*() to produce a result that is safe to copy to a PATH_MAX-sized buffer. Be careful.
This patch introduces the strbuf_addpath() function and converts a few existing users of the strbuf_addstr(git_path(...)) idiom to demonstrate the API.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- Jonathan Nieder wrote:
> I think if I ran the world, the fundamental operation would be > strbuf_addpath().
Like this, maybe.
In these v1.7.8-rc2 days, you should probably spend your time reviewing patch 2/3 of the previous series, though. ;-)
cache.h | 3 +++ notes-merge.c | 2 +- path.c | 34 ++++++++++++++++++++++++++++++++++ sha1_file.c | 2 +- transport.c | 4 ++-- 5 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/cache.h b/cache.h index 7fb85445..33d7d147 100644 --- a/cache.h +++ b/cache.h @@ -659,6 +659,8 @@ 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))); +extern void strbuf_addpath(struct strbuf *sb, const char *fmt, ...) + __attribute__((format (printf, 2, 3))); /* Return a statically allocated filename matching the sha1 signature */ extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2))); diff --git a/notes-merge.c b/notes-merge.c index 0b49e8ad..738442de 100644 --- a/notes-merge.c +++ b/notes-merge.c @@ -733,7 +733,7 @@ int notes_merge_abort(struct notes_merge_options *o) struct strbuf buf = STRBUF_INIT; int ret; - strbuf_addstr(&buf, git_path_unsafe(NOTES_MERGE_WORKTREE)); + strbuf_addpath(&buf, NOTES_MERGE_WORKTREE); OUTPUT(o, 3, "Removing notes merge worktree at %s", buf.buf); ret = remove_dir_recursively(&buf, 0); strbuf_release(&buf); diff --git a/path.c b/path.c index 0611b7be..28d6326d 100644 --- a/path.c +++ b/path.c @@ -33,6 +33,20 @@ static char *cleanup_path(char *path) return path; } +static void strbuf_cleanup_path(struct strbuf *sb, size_t pos) +{ + char *newstart; + + /* + * cleanup_path expects to be acting on a static buffer, + * so it modifies its argument in place and returns + * a pointer to the new start of the path. + */ + newstart = cleanup_path(sb->buf + pos); + strbuf_remove(sb, pos, newstart - sb->buf - pos); + strbuf_setlen(sb, strlen(sb->buf)); +} + char *mksnpath(char *buf, size_t n, const char *fmt, ...) { va_list args; @@ -68,6 +82,18 @@ bad: return buf; } +static void strbuf_vaddpath(struct strbuf *sb, const char *fmt, va_list args) +{ + const char *git_dir = get_git_dir(); + size_t pos = sb->len; + + strbuf_addstr(sb, git_dir); + if (pos < sb->len && !is_dir_sep(sb->buf[sb->len - 1])) + strbuf_addch(sb, '/'); + strbuf_vaddf(sb, fmt, args); + strbuf_cleanup_path(sb, pos); +} + char *git_snpath(char *buf, size_t n, const char *fmt, ...) { va_list args; @@ -87,6 +113,14 @@ char *git_pathdup(const char *fmt, ...) return xstrdup(path); } +void strbuf_addpath(struct strbuf *sb, const char *fmt, ...) +{ + va_list args; + va_start(args, fmt); + strbuf_vaddpath(sb, fmt, args); + va_end(args); +} + char *mkpath(const char *fmt, ...) { va_list args; diff --git a/sha1_file.c b/sha1_file.c index ba7eca89..315d1004 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -2706,7 +2706,7 @@ static int index_stream(unsigned char *sha1, int fd, size_t size, int len, tmpfd; strbuf_addstr(&export_marks, "--export-marks="); - strbuf_addstr(&export_marks, git_path_unsafe("hashstream_XXXXXX")); + strbuf_addpath(&export_marks, "hashstream_XXXXXX"); tmpfile = export_marks.buf + strlen("--export-marks="); tmpfd = git_mkstemp_mode(tmpfile, 0600); if (tmpfd < 0) diff --git a/transport.c b/transport.c index cc0ca04c..f5c95b40 100644 --- a/transport.c +++ b/transport.c @@ -203,7 +203,7 @@ static struct ref *get_refs_via_rsync(struct transport *transport, int for_push) /* copy the refs to the temporary directory */ - strbuf_addstr(&temp_dir, git_path_unsafe("rsync-refs-XXXXXX")); + strbuf_addpath(&temp_dir, "rsync-refs-XXXXXX"); if (!mkdtemp(temp_dir.buf)) die_errno ("Could not make temporary directory"); temp_dir_len = temp_dir.len; @@ -366,7 +366,7 @@ static int rsync_transport_push(struct transport *transport, /* copy the refs to the temporary directory; they could be packed. */ - strbuf_addstr(&temp_dir, git_path_unsafe("rsync-refs-XXXXXX")); + strbuf_addpath(&temp_dir, "rsync-refs-XXXXXX"); if (!mkdtemp(temp_dir.buf)) die_errno ("Could not make temporary directory"); strbuf_addch(&temp_dir, '/');
-- 1.7.8.rc2