On Mon, Dec 1, 2014 at 12:55 AM, Torsten Bögershausen <tboegi@xxxxxx> wrote:
> On 12/01/2014 04:40 AM, David Michael wrote:
>>
>> On Sun, Nov 30, 2014 at 3:16 PM, Torsten Bögershausen <tboegi@xxxxxx>
>> 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;
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html