Re: [PATCH] diff: "S_IFREG | 0644" to "(S_IFREG | 0644)" to avoid warning

On Mon, Oct 4, 2010 at 09:35, Jonathan Nieder [off-list ref] wrote:

Ævar Arnfjörð Bjarmason wrote:

quoted

-     unsigned mode = canon_mode(S_IFREG | 0644);
+     unsigned mode = canon_mode((S_IFREG | 0644));

Just curious:

#define canon_mode(mode) \
       (S_ISREG(mode) ? (S_IFREG | ce_permissions(mode)) : \
       S_ISLNK(mode) ? S_IFLNK : S_ISDIR(mode) ? S_IFDIR : S_IFGITLINK)

#define ce_permissions(mode) (((mode) & 0100) ? 0755 : 0644)

Since S_ISREG et al are macros, typically they would put their
argument in parentheses in the definition.  How are they defined
in NetBSD sys/stat.h?  What is canon_mode(S_IFREG | 0644) being
misinterpreted to mean?

Oh it's a bug in NetBSD, sorry for not being explicit about that:

    $ grep S_ISREG /usr/include/sys/stat.h
    #define S_ISREG(m)      ((m & _S_IFMT) == _S_IFREG)     /* regular file */

    $ grep S_ISREG /usr/include/linux/stat.h
    #define S_ISREG(m)      (((m) & S_IFMT) == S_IFREG)

I.e. GCC sees `S_IFREG | 0644 & _S_IFMT' on NetBSD but `(S_IFREG |
0644) & _S_IFMT' on Linux.

Since bitwise AND (&) has precedence over bitwise OR it's probably a
logic error on NetBSD too, not just an annoying warning.