Re: [PATCH] strbuf_readlink semantics update.

[Pierre Habouzit <madcoder@debian.org>]

[-- Attachment #1: Type: text/plain, Size: 4021 bytes --]

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@debian.org>
---

  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 #2: Type: application/pgp-signature, Size: 197 bytes --]