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 <madcoder@xxxxxxxxxx>
---
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
Attachment:
pgpwmvDfIhlnG.pgp
Description: PGP signature
