All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: David Michael <fedora.dm0@gmail.com>
Cc: Duy Nguyen <pclouds@gmail.com>, Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH] compat: convert modes to use portable file type values
Date: Mon, 01 Dec 2014 09:57:16 -0800	[thread overview]
Message-ID: <xmqqsigzp81v.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CAEvUa7mhjG1xPoJedp4XYrxr39_EuzvGtONLv0B=uBw+vQB5pw@mail.gmail.com> (David Michael's message of "Mon, 1 Dec 2014 12:49:59 -0500")

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?

  reply	other threads:[~2014-12-01 17:57 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-30  2:41 [PATCH] compat: convert modes to use portable file type values David Michael
2014-11-30 20:16 ` Torsten Bögershausen
2014-12-01  3:40   ` David Michael
2014-12-01  5:55     ` Torsten Bögershausen
2014-12-01 12:48       ` David Michael
2014-12-01 17:46         ` David Michael
2014-12-01  3:11 ` Junio C Hamano
2014-12-01 14:44 ` Duy Nguyen
2014-12-01 17:49   ` David Michael
2014-12-01 17:57     ` Junio C Hamano [this message]
2014-12-01 19:10       ` David Michael
2014-12-01 20:09         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqsigzp81v.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=fedora.dm0@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.