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

David Michael [off-list ref] writes:

On Mon, Dec 1, 2014 at 9:44 AM, Duy Nguyen [off-list ref] wrote:

quoted

On Sun, Nov 30, 2014 at 9:41 AM, David Michael [off-list ref] wrote:

quoted

+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?

On Mon, Dec 1, 2014 at 12:57 PM, Junio C Hamano [off-list ref] wrote:

David Michael [off-list ref] writes:

quoted

On Mon, Dec 1, 2014 at 9:44 AM, Duy Nguyen [off-list ref] wrote:

quoted

On Sun, Nov 30, 2014 at 9:41 AM, David Michael [off-list ref] wrote:

quoted

+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