git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Stefan Beller <sbeller@google.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, Michael Haggerty <mhagger@alum.mit.edu>,
	Jeff King <peff@peff.net>
Subject: [PATCH 03/14] copy_fd: pass error message back through a strbuf
Date: Tue, 2 Dec 2014 21:13:44 -0800	[thread overview]
Message-ID: <20141203051344.GM6527@google.com> (raw)
In-Reply-To: <20141203050217.GJ6527@google.com>

This way, callers can put the message in context or even avoid
printing the message altogether.

Currently hold_lock_file_for_append tries to save errno in order to
produce a meaningful message about the failure and tries to print a
second message using errno.  Unfortunately the errno it uses is not
meaningful because copy_fd makes no effort to set a meaningful errno
value.  This change is preparation for simplifying the
hold_lock_file_for_append behavior.

No user-visible change intended yet.

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

 cache.h    |  2 +-
 convert.c  |  6 +++++-
 copy.c     | 32 +++++++++++++++++++++-----------
 lockfile.c |  6 +++++-
 4 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/cache.h b/cache.h
index 99ed096..ddaa30f 100644
--- a/cache.h
+++ b/cache.h
@@ -1479,7 +1479,7 @@ extern const char *git_mailmap_blob;
 extern void maybe_flush_or_die(FILE *, const char *);
 __attribute__((format (printf, 2, 3)))
 extern void fprintf_or_die(FILE *, const char *fmt, ...);
-extern int copy_fd(int ifd, int ofd);
+extern int copy_fd(int ifd, int ofd, struct strbuf *err);
 extern int copy_file(const char *dst, const char *src, int mode);
 extern int copy_file_with_time(const char *dst, const char *src, int mode);
 extern void write_or_die(int fd, const void *buf, size_t count);
diff --git a/convert.c b/convert.c
index 9a5612e..e301447 100644
--- a/convert.c
+++ b/convert.c
@@ -358,7 +358,11 @@ static int filter_buffer_or_fd(int in, int out, void *data)
 	if (params->src) {
 		write_err = (write_in_full(child_process.in, params->src, params->size) < 0);
 	} else {
-		write_err = copy_fd(params->fd, child_process.in);
+		struct strbuf err = STRBUF_INIT;
+		write_err = copy_fd(params->fd, child_process.in, &err);
+		if (write_err)
+			error("copy-fd: %s", err.buf);
+		strbuf_release(&err);
 	}
 
 	if (close(child_process.in))
diff --git a/copy.c b/copy.c
index f2970ec..828661a 100644
--- a/copy.c
+++ b/copy.c
@@ -1,19 +1,22 @@
 #include "cache.h"
 
-int copy_fd(int ifd, int ofd)
+int copy_fd(int ifd, int ofd, struct strbuf *err)
 {
+	assert(err);
+
 	while (1) {
 		char buffer[8192];
 		ssize_t len = xread(ifd, buffer, sizeof(buffer));
 		if (!len)
 			break;
 		if (len < 0) {
-			return error("copy-fd: read returned %s",
-				     strerror(errno));
+			strbuf_addf(err, "read returned %s", strerror(errno));
+			return -1;
+		}
+		if (write_in_full(ofd, buffer, len) < 0) {
+			strbuf_addf(err, "write returned %s", strerror(errno));
+			return -1;
 		}
-		if (write_in_full(ofd, buffer, len) < 0)
-			return error("copy-fd: write returned %s",
-				     strerror(errno));
 	}
 	return 0;
 }
@@ -33,7 +36,8 @@ static int copy_times(const char *dst, const char *src)
 
 int copy_file(const char *dst, const char *src, int mode)
 {
-	int fdi, fdo, status;
+	int fdi, fdo;
+	struct strbuf err = STRBUF_INIT;
 
 	mode = (mode & 0111) ? 0777 : 0666;
 	if ((fdi = open(src, O_RDONLY)) < 0)
@@ -42,15 +46,21 @@ int copy_file(const char *dst, const char *src, int mode)
 		close(fdi);
 		return fdo;
 	}
-	status = copy_fd(fdi, fdo);
+	if (copy_fd(fdi, fdo, &err)) {
+		close(fdi);
+		close(fdo);
+		error("copy-fd: %s", err.buf);
+		strbuf_release(&err);
+		return -1;
+	}
+	strbuf_release(&err);
 	close(fdi);
 	if (close(fdo) != 0)
 		return error("%s: close error: %s", dst, strerror(errno));
-
-	if (!status && adjust_shared_perm(dst))
+	if (adjust_shared_perm(dst))
 		return -1;
 
-	return status;
+	return 0;
 }
 
 int copy_file_with_time(const char *dst, const char *src, int mode)
diff --git a/lockfile.c b/lockfile.c
index 4f16ee7..b3020f3 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -182,6 +182,7 @@ 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)
 {
 	int fd, orig_fd;
+	struct strbuf err = STRBUF_INIT;
 
 	fd = lock_file(lk, path, flags);
 	if (fd < 0) {
@@ -202,9 +203,11 @@ int hold_lock_file_for_append(struct lock_file *lk, const char *path, int flags)
 			errno = save_errno;
 			return -1;
 		}
-	} else if (copy_fd(orig_fd, fd)) {
+	} else if (copy_fd(orig_fd, fd, &err)) {
 		int save_errno = errno;
 
+		error("copy-fd: %s", err.buf);
+		strbuf_release(&err);
 		if (flags & LOCK_DIE_ON_ERROR)
 			exit(128);
 		close(orig_fd);
@@ -214,6 +217,7 @@ int hold_lock_file_for_append(struct lock_file *lk, const char *path, int flags)
 	} else {
 		close(orig_fd);
 	}
+	strbuf_release(&err);
 	return fd;
 }
 

  parent reply	other threads:[~2014-12-03  5:13 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-17 22:14 [PATCH] copy.c: make copy_fd preserve meaningful errno Stefan Beller
2014-11-17 23:08 ` Junio C Hamano
2014-11-17 23:13   ` Stefan Beller
2014-11-17 23:35 ` Jonathan Nieder
2014-11-18  0:18   ` Stefan Beller
2014-11-18  0:48     ` Jonathan Nieder
2014-11-18  1:01       ` Stefan Beller
2014-12-03  5:02         ` [PATCH 0/14] " Jonathan Nieder
2014-12-03  5:10           ` [PATCH 01/14] strbuf: introduce strbuf_prefixf() Jonathan Nieder
2014-12-03 20:10             ` Eric Sunshine
2014-12-03 20:14               ` Jonathan Nieder
2014-12-03 21:45             ` Junio C Hamano
2014-12-03 21:59               ` Jonathan Nieder
2014-12-03 22:09                 ` Jonathan Nieder
2014-12-03 22:40                 ` Junio C Hamano
2014-12-03  5:12           ` [PATCH 02/14] add_to_alternates_file: respect GIT_OBJECT_DIRECTORY Jonathan Nieder
2014-12-03 18:53             ` Junio C Hamano
2014-12-03  5:13           ` Jonathan Nieder [this message]
2014-12-03 19:01             ` [PATCH 03/14] copy_fd: pass error message back through a strbuf Junio C Hamano
2014-12-03 20:28               ` Jonathan Nieder
2014-12-03 21:00               ` Jeff King
2014-12-03 21:38                 ` Jonathan Nieder
2014-12-04  7:59                   ` Jeff King
2014-12-04  8:36                     ` Stefan Beller
2014-12-04  9:04                       ` Jeff King
2014-12-10 17:02                     ` Michael Haggerty
2014-12-10 19:00                       ` Junio C Hamano
2014-12-10 19:14                         ` Jeff King
2014-12-04  3:01               ` [PATCH/RFC] doc: document error handling functions and conventions (Re: [PATCH 03/14] copy_fd: pass error message back through a strbuf) Jonathan Nieder
2014-12-04 23:27                 ` Junio C Hamano
2014-12-04 23:37                   ` Jonathan Nieder
2014-12-04 23:41                   ` Jonathan Nieder
2014-12-04 23:44                     ` Jeff King
2014-12-04 23:52                       ` Junio C Hamano
2014-12-05  0:01                         ` Jeff King
2014-12-05 18:00                           ` Junio C Hamano
2014-12-07 10:07                             ` Jeff King
2014-12-09 18:43                               ` Junio C Hamano
2014-12-09 18:49                                 ` Jeff King
2015-02-12 23:08                                 ` Junio C Hamano
2015-02-17 15:50                                   ` Michael Haggerty
2015-02-17 16:03                                     ` Junio C Hamano
2015-02-17 16:05                                       ` Jeff King
2015-02-17 22:46                                         ` Junio C Hamano
2014-12-03 20:02             ` [PATCH 03/14] copy_fd: pass error message back through a strbuf Junio C Hamano
2014-12-03 20:18               ` Jonathan Nieder
2014-12-03 21:43                 ` Junio C Hamano
2014-12-03 21:51                   ` Jonathan Nieder
2014-12-03 20:20               ` Stefan Beller
2014-12-03  5:14           ` [PATCH 04/14] hold_lock_file_for_append: " Jonathan Nieder
2014-12-03  6:09             ` Torsten Bögershausen
2014-12-03  7:04               ` Jonathan Nieder
2014-12-03  5:16           ` [PATCH 05/14] lock_packed_refs: " Jonathan Nieder
2014-12-03  5:19           ` [PATCH 06/14] lockfile: introduce flag for locks outside .git Jonathan Nieder
2014-12-03 23:13             ` Junio C Hamano
2014-12-03 23:24               ` Jonathan Nieder
2014-12-03 23:25               ` Junio C Hamano
2014-12-03 23:29                 ` Jonathan Nieder
2014-12-03 23:38                   ` Junio C Hamano
2014-12-03 23:41                     ` Jonathan Nieder
2014-12-03 23:43                     ` Junio C Hamano
2014-12-03 23:57                   ` Jeff King
2014-12-04  5:51                   ` Junio C Hamano
2014-12-04 17:56                     ` Jonathan Nieder
2014-12-03  5:19           ` [PATCH 07/14] fast-import: use message from lockfile API when writing marks fails Jonathan Nieder
2014-12-03  5:20           ` [PATCH 08/14] credentials: use message from lockfile API when locking ~/.git-credentials fails Jonathan Nieder
2014-12-03  5:21           ` [PATCH 09/14] config: use message from lockfile API when locking config file fails Jonathan Nieder
2014-12-03 19:59             ` Junio C Hamano
2014-12-03 20:16               ` Jonathan Nieder
2014-12-03  5:22           ` [PATCH 10/14] rerere: error out on autoupdate failure Jonathan Nieder
2014-12-03  5:25           ` [PATCH 11/14] hold_locked_index: pass error message back through a strbuf Jonathan Nieder
2014-12-03  5:26           ` [PATCH 12/14] hold_lock_file_for_update: " Jonathan Nieder
2014-12-03 18:53             ` Jonathan Nieder
2014-12-03  5:26           ` [PATCH 13/14] lockfile: remove unused function 'unable_to_lock_die' Jonathan Nieder
2014-12-03  5:27           ` [PATCH 14/14] lockfile: make 'unable_to_lock_message' private Jonathan Nieder
2014-12-03 20:42             ` Stefan Beller
2014-11-18 16:32   ` [PATCH] copy.c: make copy_fd preserve meaningful errno Junio C Hamano
2014-11-18 17:08     ` Junio C Hamano
2014-11-21  9:14 ` Michael Haggerty
2014-11-21  9:17   ` Michael Haggerty
2014-11-21 17:48     ` Junio C Hamano
2014-11-21 17:54       ` Jeff King
2014-11-21 18:31         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141203051344.GM6527@google.com \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mhagger@alum.mit.edu \
    --cc=peff@peff.net \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).