Re: [PATCH] copy.c: make copy_fd preserve meaningful errno
- From
Michael Haggerty <mhagger@alum.mit.edu>
- Date
- Nov 21, 2014, 09:14 UTC
- Message-ID
- <546F0284.7050904@alum.mit.edu>
- In-Reply-To
- <1416262453-30349-1-git-send-email-sbeller@google.com>
On 11/17/2014 11:14 PM, Stefan Beller wrote:
Show 51 quoted lines
> From: Ronnie Sahlberg <sahlberg@google.com>
>
> Update copy_fd to return a meaningful errno on failure and also
> preserve the existing errno variable.
>
> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> As announced in [1], I'm going to take over the
> ref-transactions-reflog series by Ronnie Sahlberg.
>
> This patch was sent previously to the list as part of
> that series[2], but it seems to be unrelated to me.
>
> Thanks,
> Stefan
>
> [1] http://www.mail-archive.com/git@vger.kernel.org/msg61051.html
> [2] http://www.spinics.net/lists/git/msg240784.html
> copy.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/copy.c b/copy.c
> index f2970ec..a8d366e 100644
> --- a/copy.c
> +++ b/copy.c
> @@ -8,12 +8,17 @@ int copy_fd(int ifd, int ofd)
> if (!len)
> break;
> if (len < 0) {
> - return error("copy-fd: read returned %s",
> - strerror(errno));
> + int save_errno = errno;
> + error("copy-fd: read returned %s", strerror(errno));
> + errno = save_errno;
> + return -1;
> + }
> + if (write_in_full(ofd, buffer, len) < 0) {
> + int save_errno = errno;
> + error("copy-fd: write returned %s", strerror(errno));
> + errno = save_errno;
> + return -1;
> }
> - if (write_in_full(ofd, buffer, len) < 0)
> - return error("copy-fd: write returned %s",
> - strerror(errno));
> }
> return 0;
> }
> Couldn't we save ourselves a lot of this "save_errno" boilerplate by making error() and warning() preserve errno? They don't do any meaningful internal error checking anyway, so even if they overwrite errno, that value is useless to callers (who undoubtedly wouldn't check such a value anyway). Something like
int error(const char *err, ...)
{
va_list params;
+ int save_errno = errno;va_start(params, err); error_routine(err, params); va_end(params); + errno = save_errno; return -1; }
and the same for warning()?
Michael
-- Michael Haggerty mhagger@alum.mit.edu