Thread (2 messages) flat view 2 messages, 2 authors, 2016-06-15

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:49:42

Possibly related (same subject, not in this thread)

Ævar Arnfjörð Bjarmason [off-list ref] writes:
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.
In that case, I'd prefer to work this around at the definition of
canon_mode(), like

    /*
     * extra ()-pair around S_ISREG() and friends to work around platform
     * header with buggy definitions like
     * #define S_ISREG(x) ((x & _S_IFMT) == _S_IFREG)
     */
    #define canon_mode(mode) \
    	(S_ISREG((mode)) ? (S_IFREG | ce_permissions(mode)) : \
        ...

instead of contaminating the calling sites.  Otherwise new calling sites
we will add in the future need to be aware of the same bug for no good
reason.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help