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

On Mon, Dec 1, 2014 at 12:55 AM, Torsten Bögershausen [off-list ref] wrote:

On 12/01/2014 04:40 AM, David Michael wrote:

quoted

On Sun, Nov 30, 2014 at 3:16 PM, Torsten Bögershausen [off-list ref]
wrote:
[snip]

quoted

Could the code be more human-readable ?
static inline mode_t mode_native_to_git(mode_t native_mode)
{
         int perm_bits = native_mode & 07777;
         if (S_ISREG(native_mode))
                 return 0100000 | perm_bits;
         if (S_ISDIR(native_mode))
                 return 0040000 | perm_bits;
         if (S_ISLNK(native_mode))
                 return 0120000 | perm_bits;
         if (S_ISBLK(native_mode))
                 return 0060000 | perm_bits;
         if (S_ISCHR(native_mode))
                 return 0020000 | perm_bits;
         if (S_ISFIFO(native_mode))
                 return 0010000 | perm_bits;
         /* Non-standard type bits were given. */
         /* Shouldn't we die() here ?? */
                 return perm_bits;
}

Sure, I can send an updated patch with the new variable and without the
"else"s.

Regarding the question in the last comment:  I was assuming if this
case was ever reached, Git would handle the returned mode the same way
as if it encountered an unknown/non-standard file type on a normal
operating system, which could die() if needed in the function that
called stat().

Should I send an updated patch that die()s there?

David

Not yet, please wait with a V2 patch until I finished my thinking ;-)

I take back the suggestion with the die(). I was thinking how to handle
unforeseen types, which may show up on the z/OS some day,
So die() is not a good idea, it is better to ignore them, what the code
does.

Knowing that Git does not track block devices, nor character devices nor
sockets,
the above code could be simplyfied even more, by mapping everything which
is not a directory, a file or a softlink to "device type 0)

This is just a suggestion, I want to here from others as well:

        int perm_bits = native_mode & 07777;
        if (S_ISREG(native_mode))
                return 0100000 | perm_bits;
        if (S_ISDIR(native_mode))
                return 0040000 | perm_bits;
        if (S_ISLNK(native_mode))
                return 0120000 | perm_bits;
        /* Git does not track S_IFCHR, S_IFBLK, S_IFIFO, S_IFSOCK  */
                return perm_bits;

I had considered omitting those three as well at first, but in this
case it would mean that they will be unusable all together.

The z/OS S_IFMT definition (i.e. the file type bit mask) is
0xFF000000, and the common/translated S_IFMT definition is 0xF000.
Since the S_ISxxx macros use the typical ((mode & S_IFMT) == S_IFxxx)
definition, they would never match a native z/OS mode after redefining
S_IFMT.

So translating those types isn't just for tracking files, it's to
support any use of S_ISxxx anywhere in the code.  It should be okay to
remove any of those types if we know that Git will never need to use
them.

David