On Mon, Feb 13, 2017 at 10:34:06AM -0800, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy <pclo...@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
