git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Torsten Bögershausen" <tboegi@web.de>
To: "David Michael" <fedora.dm0@gmail.com>,
	"Torsten Bögershausen" <tboegi@web.de>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: [PATCH] compat: convert modes to use portable file type values
Date: Mon, 01 Dec 2014 06:55:41 +0100	[thread overview]
Message-ID: <547C02DD.2010606@web.de> (raw)
In-Reply-To: <CAEvUa7=R7Bm7e=HCuw7QnMPwhunjJ4LUYv4LzMHMugh-iUkt8A@mail.gmail.com>

On 12/01/2014 04:40 AM, David Michael wrote:
> 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;

  reply	other threads:[~2014-12-01  5: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 [this message]
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
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=547C02DD.2010606@web.de \
    --to=tboegi@web.de \
    --cc=fedora.dm0@gmail.com \
    --cc=git@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).