Re: [PATCH] compat: convert modes to use portable file type values
- From
David Michael <fedora.dm0@gmail.com>
- Date
- Dec 1, 2014, 19:10 UTC
- Message-ID
- <CAEvUa7nsz1UX5+qakPGW=wTR_VRJU3r+VkgVt9qiCDUz30MweA@mail.gmail.com>
- In-Reply-To
- <xmqqsigzp81v.fsf@gitster.dls.corp.google.com>
On Mon, Dec 1, 2014 at 12:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 32 quoted lines
> David Michael <fedora.dm0@gmail.com> writes:
>
>> 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?Alright, it wasn't immediately clear to me from the OpenGroup page on stat() if that would always be safe. I will just test the return code in v2.
Thanks.
David