Re: [PATCH] clean: use warning_errno() when appropriate

[Jeff King <peff@peff.net>]

On Mon, Feb 13, 2017 at 10:34:06AM -0800, Junio C Hamano wrote:

> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
> 
> > All these warning() calls are preceded by a system call. Report the
> > actual error to help the user understand why we fail to remove
> > something.
> 
> I think this patch is probably correct in the current code, but I
> say this only after following what quote_path_relative() and
> relative_path() that is called from it.  These warnings are preceded
> by a call to a system library function, but it is not apparent that
> they are immediately preceded without anything else that could have
> failed in between.
> 
>     Side note.  There are many calls into strbuf API in these two
>     functions.  Any calls to xmalloc() and friends made by strbuf
>     functions may see ENOMEM from underlying malloc() and recover by
>     releasing cached resources, by which time the original errno is
>     unrecoverable.  So the above "probably correct" is not strictly
>     true.
> 
> If we care deeply enough that we want to reliably show the errno we
> got from the preceding call to a system library function even after
> whatever comes in between, I think you'd need the usual saved_errno
> trick.  Is that worth it?---I do not offhand have an opinion.

I wonder if xmalloc() should be the one doing the saved_errno trick.
After all, it has only two outcomes: we successfully allocated the
memory, or we called die().

And that would transitively make most of the strbuf calls errno-safe
(except for obvious syscall-related ones like strbuf_read_file). And in
turn that makes quote_path_relative() pretty safe (at least when writing
to a strbuf).

-Peff