Re: [PATCH] compat: convert modes to use portable file type values

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 1, 2014, 17:57 UTC
Message-ID: <xmqqsigzp81v.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CAEvUa7mhjG1xPoJedp4XYrxr39_EuzvGtONLv0B=uBw+vQB5pw@mail.gmail.com>

David Michael <fedora.dm0@gmail.com> writes:

Show 17 quoted lines

> On Mon, Dec 1, 2014 at 9:44 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Sun, Nov 30, 2014 at 9:41 AM, David Michael <fedora.dm0@gmail.com> wrote:
>>> +int git_stat(const char *path, struct stat *buf)
>>> +{
>>> +       int rc;
>>> +       rc = stat(path, buf);
>>> +       if (buf != NULL)
>>
>> It's a minor thing, but maybe test "!rc" instead of "buf != NULL"?
>
> Okay, it makes sense to only do the conversion for a successful return code.
>
> Should it test for both a zero return code and a non-null pointer?  I
> don't know if there are any cases where passing a null pointer is
> legal.  The standard doesn't seem to explicitly forbid it.  z/OS
> returns -1 and sets errno to EFAULT when stat() is given NULL, but
> this patch should be able to be used on any platform.

Huh? I am confused. Since when is it legal to give NULL as statbuf to (l)stat(2)?

Wouldn't something like this be sufficient and necessary?

	int rc = stat(path, buf);
        if (rc)
		return rc;

That is, let the underlying stat(2) diagnose any and all problems (and leave clues in errno) and parrot its return value to the caller to signal the failure?