On Di, 2014-12-09 at 07:21 +0100, Max Kellermann wrote:
> On 2014/12/09 00:10, Ben Boeckel <maths...@gmail.com> wrote:
> > On Mon, Dec 08, 2014 at 22:14:16 +0100, Jörg Krause wrote:
> > > FYI: This is the statement of a musl maintainer about this issue:
> > > http://www.openwall.com/lists/musl/2014/12/08/15
> > 
> > That is???unfortunate. Macros are basically evil and with this kind of
> > attitude, anytime I use a standard C library, POSIX, etc. function from
> > C++, I have to first #undef it to make sure I get the right one to be
> > "portable" :/ . IMO, if that's the level of burden to be portable to
> > your C library, you can keep your patches (nb. I'm not an mpd dev, so
> > this opinion isn't binding here). Not to mention that macros have
> > approximately zero support for usage inside of C++'s std algorithms (or
> > C's limited selection for that matter).
> 
> I agree.  And I believe Rich's opinion on this is wrong.  Not
> formally/technically wrong, just wrong as in "I disagree that a
> standard library developer should decide that way".  Even if something
> is formally legal according to some specification, some common sense
> must be applied.
> 
> Macros should be avoided whenever possible, and inline functions are
> almost always the better replacement.
> 
> Just look at this MUSL code that broke the MPD build:
> 
>  int pthread_equal(pthread_t, pthread_t);
>  #define pthread_equal(x,y) ((x)==(y))
> 
> That macro is supposed to optimize some pthread_equal() calls.  This
> is how it should be done:
> 
>  static inline int pthread_equal(pthread_t a, pthread_t b) {
>     return a == b;
>  }
> 
> This is C code, while the former is NOT C code - it escapes from the
> "raw" C language into the preprocessor language.  My version is
> type-safe.  And safe with C++.  It is longer, but more readable, and
> comes without the paranthese hacks necessary in macros.  (And there
> are many more advantages.)  Both generate the same machine code.

First, inline is only a hint to the compiler to optimize the function.

Second, the static keyword on the function forces the inline function to
have an internal linkage. Each instance of such a function is treated as
a separate function (address of each function is different) :

$ cat foo.h
        static inline int foo() { return 3; }

$ cat test.c
        #include <stdio.h>
        #include "foo.h"
        void g() {
           printf("foo called from g: return value = %d, address = %#p
        \n", foo(), &foo);
        }

$ cat main.c
        #include <stdio.h>
        #include "foo.h"
        
        void g();
        
        int main() {
           printf("foo called from main: return value = %d, address = %
        #p\n", foo(), &foo);
           g();
        }

$ gcc main.c test.c && ./a.out
        foo called from main: return value = 3, address = 0x400506
        foo called from g: return value = 3, address = 0x400541
        
So internal linkage is not desirable for a C library where you possibly
have dozens of translations units, eg in a linux distribution.

> 
> It beats me why MUSL developers thought a macro is better; that is so
> obviously wrong that I don't know what to say.  Inexperienced C
> developers often prefer macros over inline functions, as did I decades
> ago when I was new to C.
> 
> I agree with Ben, and my motivation to keep MPD compatible with a
> badly designed standard library is not big.
> 

I wouldn't say that musl is a badly designed library. It's not free of
bugs, and some bevaviour may be unexpected, but the source code is much
more clean then glibc.

Furthermore musl supports MMU-less systems and compiles with much
smaller binary size. 

> And no, MPD will not add workarounds for MUSL (#undef).  Removing "::"
> was barely acceptable, even though I don't like that change.

Maybe it was a mistake to first sent the patches to MPD instead of
asking the musl maintainers about the macro issue. As Ben has already
posted the musl people already changed the behaviour of the macro
definitions.

_______________________________________________
mpd-devel mailing list
mpd-devel@musicpd.org
http://mailman.blarg.de/listinfo/mpd-devel

Reply via email to