Re: [PATCH 01/14] strbuf: introduce strbuf_prefixf()

[Jonathan Nieder <jrnieder@gmail.com>]

Jonathan Nieder wrote:
> Junio C Hamano wrote:

>> In any case, instead of this:
>>
>> 	struct strbuf tc_err = STRBUF_INIT;
>>       if (transaction_commit(&t, &tc_err)) {
>> 		strbuf_addf(err, "cannot fetch '%s': %s", remotename,
>> 			tc_err.buf);
>> 		strbuf_release(&tc_err);
>> 		return -1;
>> 	}
>>
>> you can use the four-line version you cited above, which might be an
>> improvement.
>
> Yes, that's the idea.
>
> I'll do the tc_err thing, since I'm not getting a clear feeling that
> I've offered enough motivation for the prefixf approach.

The tc_err approach looks like this.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

 lockfile.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git i/lockfile.c w/lockfile.c
index 8685c68..4e2dfa3 100644
--- i/lockfile.c
+++ w/lockfile.c
@@ -182,15 +182,16 @@ int hold_lock_file_for_update(struct lock_file *lk, const char *path, int flags)
 int hold_lock_file_for_append(struct lock_file *lk, const char *path,
 			      int flags, struct strbuf *err)
 {
-	int fd, orig_fd;
+	struct strbuf tc_err = STRBUF_INIT;
+	int fd, orig_fd = -1;
 
 	assert(!(flags & LOCK_DIE_ON_ERROR));
-	assert(err && !err->len);
+	assert(err);
 
 	fd = lock_file(lk, path, flags);
 	if (fd < 0) {
 		unable_to_lock_message(path, errno, err);
-		return fd;
+		goto fail;
 	}
 
 	orig_fd = open(path, O_RDONLY);
@@ -198,18 +199,22 @@ int hold_lock_file_for_append(struct lock_file *lk, const char *path,
 		if (errno != ENOENT) {
 			strbuf_addf(err, "cannot open '%s' for copying: %s",
 				    path, strerror(errno));
-			rollback_lock_file(lk);
-			return -1;
+			goto fail;
 		}
-	} else if (copy_fd(orig_fd, fd, err)) {
-		strbuf_prefixf(err, "cannot copy '%s': ", path);
-		close(orig_fd);
-		rollback_lock_file(lk);
-		return -1;
+	} else if (copy_fd(orig_fd, fd, &tc_err)) {
+		strbuf_addf(err, "cannot copy '%s': %s", path, tc_err.buf);
+		goto fail;
 	} else {
 		close(orig_fd);
 	}
+	strbuf_release(&tc_err);
 	return fd;
+fail:
+	if (orig_fd >= 0)
+		close(orig_fd);
+	rollback_lock_file(lk);
+	strbuf_release(&tc_err);
+	return -1;
 }
 
 FILE *fdopen_lock_file(struct lock_file *lk, const char *mode)