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

From: Torsten Bögershausen <tboegi@web.de>
Date: Dec 1, 2014, 05:55 UTC
Message-ID: <547C02DD.2010606@web.de>
In-Reply-To: <CAEvUa7=R7Bm7e=HCuw7QnMPwhunjJ4LUYv4LzMHMugh-iUkt8A@mail.gmail.com>

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

Show 33 quoted lines

> On Sun, Nov 30, 2014 at 3:16 PM, Torsten Bögershausen <tboegi@web.de> wrote:
> [snip]
>> 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;