when readlink fails, the strbuf shall not be destroyed. It's not how read_file_or_gitlink works for example. Fix strbuf_readlink callers to destroy the buffer when appropriate. Fix read_old_data possible leaks in case of errors, since even when no data has been read, the strbufs may have grown to prepare the reads. strbuf_release must be called on them. Signed-off-by: Pierre Habouzit --- I know it somehow add lines to the callers, but it's actually more important to keep consistency among the strbuf APIs than to save 10 SLOCs. builtin-apply.c | 8 ++++++-- combine-diff.c | 1 + diff.c | 4 +++- read-cache.c | 4 +++- sha1_file.c | 1 + strbuf.c | 2 +- 6 files changed, 15 insertions(+), 5 deletions(-) diff --git a/builtin-apply.c b/builtin-apply.c index 07244b0..c1fe9ca 100644 --- a/builtin-apply.c +++ b/builtin-apply.c @@ -2306,8 +2306,10 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry * /* We have a patched copy in memory use that */ strbuf_add(&buf, tpatch->result, tpatch->resultsize); } else if (cached) { - if (read_file_or_gitlink(ce, &buf)) + if (read_file_or_gitlink(ce, &buf)) { + strbuf_release(&buf); return error("read of %s failed", patch->old_name); + } } else if (patch->old_name) { if (S_ISGITLINK(patch->old_mode)) { if (ce) { @@ -2320,8 +2322,10 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry * patch->fragments = NULL; } } else { - if (read_old_data(st, patch->old_name, &buf)) + if (read_old_data(st, patch->old_name, &buf)) { + strbuf_release(&buf); return error("read of %s failed", patch->old_name); + } } } diff --git a/combine-diff.c b/combine-diff.c index bccc018..674745d 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -706,6 +706,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent, struct strbuf buf = STRBUF_INIT; if (strbuf_readlink(&buf, elem->path, st.st_size) < 0) { + strbuf_release(&buf); error("readlink(%s): %s", elem->path, strerror(errno)); return; diff --git a/diff.c b/diff.c index b57d9ac..41f7e1c 100644 --- a/diff.c +++ b/diff.c @@ -1778,8 +1778,10 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only) if (S_ISLNK(st.st_mode)) { struct strbuf sb = STRBUF_INIT; - if (strbuf_readlink(&sb, s->path, s->size)) + if (strbuf_readlink(&sb, s->path, s->size)) { + strbuf_release(&sb); goto err_empty; + } s->size = sb.len; s->data = strbuf_detach(&sb, NULL); s->should_free = 1; diff --git a/read-cache.c b/read-cache.c index db166da..9673d91 100644 --- a/read-cache.c +++ b/read-cache.c @@ -104,8 +104,10 @@ static int ce_compare_link(struct cache_entry *ce, size_t expected_size) enum object_type type; struct strbuf sb = STRBUF_INIT; - if (strbuf_readlink(&sb, ce->name, expected_size)) + if (strbuf_readlink(&sb, ce->name, expected_size)) { + strbuf_release(&sb); return -1; + } buffer = read_sha1_file(ce->sha1, &type, &size); if (buffer) { diff --git a/sha1_file.c b/sha1_file.c index 52d1ead..a62b53d 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -2538,6 +2538,7 @@ int index_path(unsigned char *sha1, const char *path, struct stat *st, int write case S_IFLNK: if (strbuf_readlink(&sb, path, st->st_size)) { char *errstr = strerror(errno); + strbuf_release(&sb); return error("readlink(\"%s\"): %s", path, errstr); } diff --git a/strbuf.c b/strbuf.c index 254a7ee..b1f2a97 100644 --- a/strbuf.c +++ b/strbuf.c @@ -311,7 +311,7 @@ int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint) /* .. the buffer was too small - try again */ hint *= 2; } - strbuf_release(sb); + sb->buf[sb->len] = '\0'; return -1; } -- 1.6.1.rc4.306.g849c2