patch, 5 partsBe careful about lstat()-vs-readlink()

cache.h, read-cache.c

diff --git a/cache.h b/cache.h
index 231c06d..2d85fca 100644
--- a/cache.h
+++ b/cache.h
@@ -943,4 +943,7 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix);
 char *alias_lookup(const char *alias);
 int split_cmdline(char *cmdline, const char ***argv);
 
+extern int gitlstat(const char *, struct stat *);
+#define lstat gitlstat
+
 #endif /* CACHE_H */
diff --git a/read-cache.c b/read-cache.c
index b1475ff..defbb20 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -15,6 +15,16 @@
 #include "revision.h"
 #include "blob.h"
 
+#undef lstat
+int gitlstat(const char *path, struct stat *buf)
+{
+	int retval = lstat(path, buf);
+	if (!retval && S_ISLNK(buf->st_mode))
+		buf->st_size = 1;
+	return retval;
+}
+#define lstat gitlstat
+
 /* Index extensions.
  *
  * The first letter should be 'A'..'Z' for extensions that are not

builtin-apply.c, strbuf.c, strbuf.h

diff --git a/builtin-apply.c b/builtin-apply.c
index 4c4d1e1..07244b0 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1559,10 +1559,8 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)
 {
 	switch (st->st_mode & S_IFMT) {
 	case S_IFLNK:
-		strbuf_grow(buf, st->st_size);
-		if (readlink(path, buf->buf, st->st_size) != st->st_size)
-			return -1;
-		strbuf_setlen(buf, st->st_size);
+		if (strbuf_readlink(buf, path, st->st_size) < 0)
+			return error("unable to read symlink %s", path);
 		return 0;
 	case S_IFREG:
 		if (strbuf_read_file(buf, path, st->st_size) != st->st_size)
diff --git a/strbuf.c b/strbuf.c
index 13be67e..904a2b0 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -288,6 +288,34 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, size_t hint)
 	return sb->len - oldlen;
 }
 
+#define STRBUF_MAXLINK (2*PATH_MAX)
+
+int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)
+{
+	if (hint < 32)
+		hint = 32;
+
+	while (hint < STRBUF_MAXLINK) {
+		int len;
+
+		strbuf_grow(sb, hint);
+		len = readlink(path, sb->buf, hint);
+		if (len < 0) {
+			if (errno != ERANGE)
+				break;
+		} else if (len < hint) {
+			strbuf_setlen(sb, len);
+			return 0;
+		}
+
+		/* .. the buffer was too small - try again */
+		hint *= 2;
+		continue;
+	}
+	strbuf_release(sb);
+	return -1;
+}
+
 int strbuf_getline(struct strbuf *sb, FILE *fp, int term)
 {
 	int ch;
diff --git a/strbuf.h b/strbuf.h
index b1670d9..89bd36e 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -124,6 +124,7 @@ extern size_t strbuf_fread(struct strbuf *, size_t, FILE *);
 /* XXX: if read fails, any partial read is undone */
 extern ssize_t strbuf_read(struct strbuf *, int fd, size_t hint);
 extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint);
+extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint);
 
 extern int strbuf_getline(struct strbuf *, FILE *, int);
 

diff --git a/read-cache.c b/read-cache.c
index c14b562..b1475ff 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -99,27 +99,21 @@ static int ce_compare_data(struct cache_entry *ce, struct stat *st)
 static int ce_compare_link(struct cache_entry *ce, size_t expected_size)
 {
 	int match = -1;
-	char *target;
 	void *buffer;
 	unsigned long size;
 	enum object_type type;
-	int len;
+	struct strbuf sb = STRBUF_INIT;
 
-	target = xmalloc(expected_size);
-	len = readlink(ce->name, target, expected_size);
-	if (len != expected_size) {
-		free(target);
+	if (strbuf_readlink(&sb, ce->name, expected_size))
 		return -1;
-	}
+
 	buffer = read_sha1_file(ce->sha1, &type, &size);
-	if (!buffer) {
-		free(target);
-		return -1;
+	if (buffer) {
+		if (size == sb.len)
+			match = memcmp(buffer, sb.buf, size);
+		free(buffer);
 	}
-	if (size == expected_size)
-		match = memcmp(buffer, target, size);
-	free(buffer);
-	free(target);
+	strbuf_release(&sb);
 	return match;
 }
 

diff --git a/sha1_file.c b/sha1_file.c
index 0e021c5..52d1ead 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2523,8 +2523,7 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object,
 int index_path(unsigned char *sha1, const char *path, struct stat *st, int write_object)
 {
 	int fd;
-	char *target;
-	size_t len;
+	struct strbuf sb = STRBUF_INIT;
 
 	switch (st->st_mode & S_IFMT) {
 	case S_IFREG:
@@ -2537,20 +2536,17 @@ int index_path(unsigned char *sha1, const char *path, struct stat *st, int write
 				     path);
 		break;
 	case S_IFLNK:
-		len = xsize_t(st->st_size);
-		target = xmalloc(len + 1);
-		if (readlink(path, target, len + 1) != st->st_size) {
+		if (strbuf_readlink(&sb, path, st->st_size)) {
 			char *errstr = strerror(errno);
-			free(target);
 			return error("readlink(\"%s\"): %s", path,
 			             errstr);
 		}
 		if (!write_object)
-			hash_sha1_file(target, len, blob_type, sha1);
-		else if (write_sha1_file(target, len, blob_type, sha1))
+			hash_sha1_file(sb.buf, sb.len, blob_type, sha1);
+		else if (write_sha1_file(sb.buf, sb.len, blob_type, sha1))
 			return error("%s: failed to insert into database",
 				     path);
-		free(target);
+		strbuf_release(&sb);
 		break;
 	case S_IFDIR:
 		return resolve_gitlink_ref(path, "HEAD", sha1);

diff --git a/diff.c b/diff.c
index afefe08..4b2029c 100644
--- a/diff.c
+++ b/diff.c
@@ -1773,19 +1773,17 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)
 		s->size = xsize_t(st.st_size);
 		if (!s->size)
 			goto empty;
-		if (size_only)
-			return 0;
 		if (S_ISLNK(st.st_mode)) {
-			int ret;
-			s->data = xmalloc(s->size);
-			s->should_free = 1;
-			ret = readlink(s->path, s->data, s->size);
-			if (ret < 0) {
-				free(s->data);
+			struct strbuf sb = STRBUF_INIT;
+
+			if (strbuf_readlink(&sb, s->path, s->size))
 				goto err_empty;
-			}
+			s->data = strbuf_detach(&sb, &s->size);
+			s->should_free = 1;
 			return 0;
 		}
+		if (size_only)
+			return 0;
 		fd = open(s->path, O_RDONLY);
 		if (fd < 0)
 			goto err_empty;

diff --git a/diff.c b/diff.c
index 4b2029c..f160c1a 100644
--- a/diff.c
+++ b/diff.c
@@ -1881,13 +1881,12 @@ static void prepare_temp_file(const char *name,
 		if (S_ISLNK(st.st_mode)) {
 			int ret;
 			char buf[PATH_MAX + 1]; /* ought to be SYMLINK_MAX */
-			size_t sz = xsize_t(st.st_size);
-			if (sizeof(buf) <= st.st_size)
-				die("symlink too long: %s", name);
-			ret = readlink(name, buf, sz);
+			ret = readlink(name, buf, sizeof(buf));
 			if (ret < 0)
 				die("readlink(%s)", name);
-			prep_temp_blob(temp, buf, sz,
+			if (ret == sizeof(buf))
+				die("symlink too long: %s", name);
+			prep_temp_blob(temp, buf, ret,
 				       (one->sha1_valid ?
 					one->sha1 : null_sha1),
 				       (one->sha1_valid ?

diff --git i/abspath.c w/abspath.c
index 8194ce1..649f34f 100644
--- i/abspath.c
+++ w/abspath.c
@@ -64,6 +64,8 @@ const char *make_absolute_path(const char *path)
 			len = readlink(buf, next_buf, PATH_MAX);
 			if (len < 0)
 				die ("Invalid symlink: %s", buf);
+			if (PATH_MAX <= len)
+				die("symbolic link too long: %s", buf);
 			next_buf[len] = '\0';
 			buf = next_buf;
 			buf_index = 1 - buf_index;

diff --git i/builtin-blame.c w/builtin-blame.c
index a0d6014..aae14ef 100644
--- i/builtin-blame.c
+++ w/builtin-blame.c
@@ -1996,7 +1996,6 @@ static struct commit *fake_working_tree_commit(const char *path, const char *con
 	if (!contents_from || strcmp("-", contents_from)) {
 		struct stat st;
 		const char *read_from;
-		unsigned long fin_size;
 
 		if (contents_from) {
 			if (stat(contents_from, &st) < 0)
@@ -2008,7 +2007,6 @@ static struct commit *fake_working_tree_commit(const char *path, const char *con
 				die("Cannot lstat %s", path);
 			read_from = path;
 		}
-		fin_size = xsize_t(st.st_size);
 		mode = canon_mode(st.st_mode);
 		switch (st.st_mode & S_IFMT) {
 		case S_IFREG:
@@ -2016,9 +2014,8 @@ static struct commit *fake_working_tree_commit(const char *path, const char *con
 				die("cannot open or read %s", read_from);
 			break;
 		case S_IFLNK:
-			if (readlink(read_from, buf.buf, buf.alloc) != fin_size)
+			if (strbuf_readlink(&buf, read_from, st.st_size) < 0)
 				die("cannot readlink %s", read_from);
-			buf.len = fin_size;
 			break;
 		default:
 			die("unsupported file type %s", read_from);

diff --git i/combine-diff.c w/combine-diff.c
index ec8df39..bccc018 100644
--- i/combine-diff.c
+++ w/combine-diff.c
@@ -703,15 +703,15 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 			goto deleted_file;
 
 		if (S_ISLNK(st.st_mode)) {
-			size_t len = xsize_t(st.st_size);
-			result_size = len;
-			result = xmalloc(len + 1);
-			if (result_size != readlink(elem->path, result, len)) {
+			struct strbuf buf = STRBUF_INIT;
+
+			if (strbuf_readlink(&buf, elem->path, st.st_size) < 0) {
 				error("readlink(%s): %s", elem->path,
 				      strerror(errno));
 				return;
 			}
-			result[len] = 0;
+			result_size = buf.len;
+			result = strbuf_detach(&buf, NULL);
 			elem->mode = canon_mode(st.st_mode);
 		}
 		else if (0 <= (fd = open(elem->path, O_RDONLY)) &&

diff --git a/diffcore.h b/diffcore.h
index 5b63458..16a73e6 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -30,7 +30,7 @@ struct diff_filespec {
 	void *data;
 	void *cnt_data;
 	const char *funcname_pattern_ident;
-	unsigned long size;
+	size_t size;
 	int count;               /* Reference count */
 	int xfrm_flags;		 /* for use by the xfrm */
 	int rename_used;         /* Count of rename users */

diff --git a/diff.c b/diff.c
index f160c1a..0484601 100644
--- a/diff.c
+++ b/diff.c
@@ -1778,7 +1778,8 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)
 
 			if (strbuf_readlink(&sb, s->path, s->size))
 				goto err_empty;
-			s->data = strbuf_detach(&sb, &s->size);
+			s->size = sb.len;
+			s->data = strbuf_detach(&sb, NULL);
 			s->should_free = 1;
 			return 0;
 		}

diff --git a/diff.c b/diff.c
index f160c1a..0484601 100644
--- a/diff.c
+++ b/diff.c
@@ -1778,7 +1778,8 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)
 
 			if (strbuf_readlink(&sb, s->path, s->size))
 				goto err_empty;
-			s->data = strbuf_detach(&sb, &s->size);
+			s->size = sb.len;
+			s->data = strbuf_detach(&sb, NULL);
 			s->should_free = 1;
 			return 0;
 		}

diff --git a/strbuf.c b/strbuf.c
index bdf4954..254a7ee 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -299,12 +299,12 @@ int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)
 		int len;
 
 		strbuf_grow(sb, hint);
-		len = readlink(path, sb->buf, hint);
+		len = readlink(path, sb->buf + sb->len, hint);
 		if (len < 0) {
 			if (errno != ERANGE)
 				break;
 		} else if (len < hint) {
-			strbuf_setlen(sb, len);
+			strbuf_setlen(sb, sb->len + len);
 			return 0;
 		}
 

builtin-apply.c, combine-diff.c, diff.c, read-cache.c, sha1_file.c, strbuf.c

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;
 }
 

diff --git a/strbuf.c b/strbuf.c
index bdf4954..6ed0684 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -256,18 +256,21 @@ size_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder,
 size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f)
 {
 	size_t res;
+	size_t oldalloc = sb->alloc;
 
 	strbuf_grow(sb, size);
 	res = fread(sb->buf + sb->len, 1, size, f);
-	if (res > 0) {
+	if (res > 0)
 		strbuf_setlen(sb, sb->len + res);
-	}
+	else if (res < 0 && oldalloc == 0)
+		strbuf_release(sb);
 	return res;
 }
 
 ssize_t strbuf_read(struct strbuf *sb, int fd, size_t hint)
 {
 	size_t oldlen = sb->len;
+	size_t oldalloc = sb->alloc;
 
 	strbuf_grow(sb, hint ? hint : 8192);
 	for (;;) {
@@ -275,7 +278,10 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, size_t hint)
 
 		cnt = xread(fd, sb->buf + sb->len, sb->alloc - sb->len - 1);
 		if (cnt < 0) {
-			strbuf_setlen(sb, oldlen);
+			if (oldalloc == 0)
+				strbuf_release(sb);
+			else
+				strbuf_setlen(sb, oldlen);
 			return -1;
 		}
 		if (!cnt)
@@ -292,6 +298,8 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, size_t hint)
 
 int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)
 {
+	size_t oldalloc = sb->alloc;
+
 	if (hint < 32)
 		hint = 32;
 
@@ -311,7 +319,8 @@ int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)
 		/* .. the buffer was too small - try again */
 		hint *= 2;
 	}
-	strbuf_release(sb);
+	if (oldalloc == 0)
+		strbuf_release(sb);
 	return -1;
 }
 

Documentation/technical/api-strbuf.txt, strbuf.c

diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt
index a8ee2fe..9a4e3ea 100644
--- a/Documentation/technical/api-strbuf.txt
+++ b/Documentation/technical/api-strbuf.txt
@@ -133,8 +133,10 @@ Functions
 
 * Adding data to the buffer
 
-NOTE: All of these functions in this section will grow the buffer as
-      necessary.
+NOTE: All of the functions in this section will grow the buffer as necessary.
+If they fail for some reason other than memory shortage and the buffer hadn't
+been allocated before (i.e. the `struct strbuf` was set to `STRBUF_INIT`),
+then they will free() it.
 
 `strbuf_addch`::
 
@@ -235,6 +237,11 @@ same behaviour as well.
 	Read the contents of a file, specified by its path. The third argument
 	can be used to give a hint about the file size, to avoid reallocs.
 
+`strbuf_readlink`::
+
+	Read the target of a symbolic link, specified by its path.  The third
+	argument can be used to give a hint about the size, to avoid reallocs.
+
 `strbuf_getline`::
 
 	Read a line from a FILE* pointer. The second argument specifies the line
diff --git a/strbuf.c b/strbuf.c
index bdf4954..6ed0684 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -256,18 +256,21 @@ size_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder,
 size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f)
 {
 	size_t res;
+	size_t oldalloc = sb->alloc;
 
 	strbuf_grow(sb, size);
 	res = fread(sb->buf + sb->len, 1, size, f);
-	if (res > 0) {
+	if (res > 0)
 		strbuf_setlen(sb, sb->len + res);
-	}
+	else if (res < 0 && oldalloc == 0)
+		strbuf_release(sb);
 	return res;
 }
 
 ssize_t strbuf_read(struct strbuf *sb, int fd, size_t hint)
 {
 	size_t oldlen = sb->len;
+	size_t oldalloc = sb->alloc;
 
 	strbuf_grow(sb, hint ? hint : 8192);
 	for (;;) {
@@ -275,7 +278,10 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, size_t hint)
 
 		cnt = xread(fd, sb->buf + sb->len, sb->alloc - sb->len - 1);
 		if (cnt < 0) {
-			strbuf_setlen(sb, oldlen);
+			if (oldalloc == 0)
+				strbuf_release(sb);
+			else
+				strbuf_setlen(sb, oldlen);
 			return -1;
 		}
 		if (!cnt)
@@ -292,6 +298,8 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, size_t hint)
 
 int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)
 {
+	size_t oldalloc = sb->alloc;
+
 	if (hint < 32)
 		hint = 32;
 
@@ -311,7 +319,8 @@ int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)
 		/* .. the buffer was too small - try again */
 		hint *= 2;
 	}
-	strbuf_release(sb);
+	if (oldalloc == 0)
+		strbuf_release(sb);
 	return -1;
 }