Re: [PATCH] copy.c: make copy_fd preserve meaningful errno

2014-11-21 Thread Michael Haggerty
On 11/21/2014 10:14 AM, Michael Haggerty wrote: Couldn't we save ourselves a lot of this save_errno boilerplate by making error() and warning() preserve errno? [...] Never mind; I see that Peff already submitted a patch to this effect. Michael -- Michael Haggerty mhag...@alum.mit.edu -- To

Re: [PATCH] copy.c: make copy_fd preserve meaningful errno

2014-11-21 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes: On 11/21/2014 10:14 AM, Michael Haggerty wrote: Couldn't we save ourselves a lot of this save_errno boilerplate by making error() and warning() preserve errno? [...] Never mind; I see that Peff already submitted a patch to this effect. My

Re: [PATCH] copy.c: make copy_fd preserve meaningful errno

2014-11-21 Thread Jeff King
On Fri, Nov 21, 2014 at 09:48:19AM -0800, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: On 11/21/2014 10:14 AM, Michael Haggerty wrote: Couldn't we save ourselves a lot of this save_errno boilerplate by making error() and warning() preserve errno? [...] Never

Re: [PATCH] copy.c: make copy_fd preserve meaningful errno

2014-11-21 Thread Junio C Hamano
Jeff King p...@peff.net writes: On Fri, Nov 21, 2014 at 09:48:19AM -0800, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: On 11/21/2014 10:14 AM, Michael Haggerty wrote: Couldn't we save ourselves a lot of this save_errno boilerplate by making error() and warning()

Re: [PATCH] copy.c: make copy_fd preserve meaningful errno

2014-11-18 Thread Junio C Hamano
Jonathan Nieder jrnie...@gmail.com writes: Do any callers care about errno? Does the function's API documentation say it will make errno meaningful on error, so people making changes to copy_fd in the future know to maintain that property? *searches* Looks like callers are:

Re: [PATCH] copy.c: make copy_fd preserve meaningful errno

2014-11-18 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes: In short I am not a very big fan of passing around strbuf *err to low level helper API functions myself. But the approach does not make things much worse than it currently is, other than code churns to pass an extra pointer around. Sorry I left the

[PATCH] copy.c: make copy_fd preserve meaningful errno

2014-11-17 Thread Stefan Beller
From: Ronnie Sahlberg sahlb...@google.com Update copy_fd to return a meaningful errno on failure and also preserve the existing errno variable. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com Signed-off-by: Stefan Beller sbel...@google.com

Re: [PATCH] copy.c: make copy_fd preserve meaningful errno

2014-11-17 Thread Junio C Hamano
Stefan Beller sbel...@google.com writes: This patch was sent previously to the list as part of that series[2], but it seems to be unrelated to me. I am fine to queue obvious and trivial bits first before the larger main course. For now I'll queue this one and also the series that has been

Re: [PATCH] copy.c: make copy_fd preserve meaningful errno

2014-11-17 Thread Stefan Beller
I am reviewing the series and about to resend it with very minor nits fixed. I just want to point out this fix is orthogonal to the series and can be picked up no matter how long the reviewing/discussion of the series goes. Thanks, Stefan On Mon, Nov 17, 2014 at 3:08 PM, Junio C Hamano

Re: [PATCH] copy.c: make copy_fd preserve meaningful errno

2014-11-17 Thread Jonathan Nieder
Hi, Stefan Beller wrote: This patch was sent previously to the list as part of that series[2], but it seems to be unrelated to me. Thanks. Good call. [...] From: Ronnie Sahlberg sahlb...@google.com Update copy_fd to return a meaningful errno on failure and also preserve the existing

Re: [PATCH] copy.c: make copy_fd preserve meaningful errno

2014-11-17 Thread Stefan Beller
On Mon, Nov 17, 2014 at 3:35 PM, Jonathan Nieder jrnie...@gmail.com wrote: Hi, Stefan Beller wrote: This patch was sent previously to the list as part of that series[2], but it seems to be unrelated to me. Thanks. Good call. [...] From: Ronnie Sahlberg sahlb...@google.com Update

Re: [PATCH] copy.c: make copy_fd preserve meaningful errno

2014-11-17 Thread Jonathan Nieder
(meta-comment: please snip out the context you are not responding to, to make reading easier) Stefan Beller wrote: On Mon, Nov 17, 2014 at 3:35 PM, Jonathan Nieder jrnie...@gmail.com wrote: Stefan Beller wrote: Update copy_fd to return a meaningful errno on failure and also preserve the

Re: [PATCH] copy.c: make copy_fd preserve meaningful errno

2014-11-17 Thread Stefan Beller
On Mon, Nov 17, 2014 at 4:48 PM, Jonathan Nieder jrnie...@gmail.com wrote: (meta-comment: please snip out the context you are not responding to, to make reading easier) will do After this patch, setting errno is not part of the contract of copy_fd, so the bug Ronnie was fixing is gone.