Compile of newer mosquitto versions fail for systems with non recent compilers

[pfichtner]

I cannot compile moquitto 1.6.15 for FritzBox, while I could compile 1.6.8 successfully. Error message is macro "pthread_testcancel" passed 1 arguments, but takes just 0

make[2]: Entering directory '/freetz/source/target-mipsel_gcc-4.6.4_uClibc-0.9.32.1/mosquitto-1.6.15/src'
/freetz/toolchain/build/mipsel_gcc-4.6.4_uClibc-0.9.32.1/mipsel-linux-uclibc/bin/mipsel-linux-uclibc-gcc  -I. -I.. -I../lib -I../src/deps -DWITH_BRIDGE -DWITH_PERSISTENCE -DWITH_SYS_TREE -DWITH_EC -DWITH_EPOLL -Ideps -march=4kc -mtune=4kc -msoft-float -Os -pipe -Wa,--trap -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 --std=gnu99 -DVERSION="\"1.6.15\"" -DWITH_BROKER -c mosquitto.c -o mosquitto.o
/freetz/toolchain/build/mipsel_gcc-4.6.4_uClibc-0.9.32.1/mipsel-linux-uclibc/bin/mipsel-linux-uclibc-gcc  -I. -I.. -I../lib -I../src/deps -DWITH_BRIDGE -DWITH_PERSISTENCE -DWITH_SYS_TREE -DWITH_EC -DWITH_EPOLL -Ideps -march=4kc -mtune=4kc -msoft-float -Os -pipe -Wa,--trap -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 --std=gnu99 -DVERSION="\"1.6.15\"" -DWITH_BROKER -c ../lib/alias_mosq.c -o alias_mosq.o
In file included from /freetz/toolchain/build/mipsel_gcc-4.6.4_uClibc-0.9.32.1/mipsel-linux-uclibc/lib/gcc/mipsel-linux-uclibc/4.6.4/../../../../mipsel-linux-uclibc/include/bits/uClibc_mutex.h:15:0,
                 from /freetz/toolchain/build/mipsel_gcc-4.6.4_uClibc-0.9.32.1/mipsel-linux-uclibc/lib/gcc/mipsel-linux-uclibc/4.6.4/../../../../mipsel-linux-uclibc/include/bits/uClibc_stdio.h:107,
                 from /freetz/toolchain/build/mipsel_gcc-4.6.4_uClibc-0.9.32.1/mipsel-linux-uclibc/lib/gcc/mipsel-linux-uclibc/4.6.4/../../../../mipsel-linux-uclibc/include/stdio.h:72,
                 from ../lib/memory_mosq.h:20,
                 from ../lib/alias_mosq.c:21:
/freetz/toolchain/build/mipsel_gcc-4.6.4_uClibc-0.9.32.1/mipsel-linux-uclibc/lib/gcc/mipsel-linux-uclibc/4.6.4/../../../../mipsel-linux-uclibc/include/pthread.h:622:37: error: macro "pthread_testcancel" passed 1 arguments, but takes just 0
make[2]: *** [Makefile:82: alias_mosq.o] Error 1

uClibc's pthread.h in the version 0.9.32.1 defines extern void pthread_testcancel (void); while mosquitto's dummythread.h does #define pthread_testcancel()

Patching moquitto's dummypthread.h resolved the problem

--- lib/dummypthread.h	2021-06-09 16:06:23.000000000 +0200
+++ lib/dummypthread.h	2021-12-30 11:56:21.898242877 +0100
@@ -4,7 +4,7 @@
 #define pthread_create(A, B, C, D)
 #define pthread_join(A, B)
 #define pthread_cancel(A)
-#define pthread_testcancel()
+#define pthread_testcancel(void)
 
 #define pthread_mutex_init(A, B)
 #define pthread_mutex_destroy(A)

Other binaries on the box are compiled by the vendor and cannot be recompiled since they are closed source. So an upgrade of uClibc is not an option. Could you please add support for those older uClibc versions? Thanks you!

[pfichtner]

After some googling it seems that the problem is, that there is indeed a difference in the definition in pthread's pthread.h and mosquitto's dummythread.h. While pthread and ulibc(-ng) define pthread_testcancel(void) (both in recent versions) mosquitto's dummythread.h defines pthread_testcancel() My problem seems to be, that on recent compiler versions this is "fixed" by the compiler while on my system where a very old compiler is used this is not going to work.

[ptjm]

There's no problem with the definition in dummythread. The problem is that your system imports pthread.h after dummythread has tired to stub out all the pthread methods.

What the error is saying is that you've got a macro defined to take no arguments, but you're calling it with an argument. The "call" in this case is actually intended to be the definition of a C function of the same name which takes no arguments. You can make it stop failing at that point by definition the macro to take an argument, but then the macro definition won't match the C function definition, and I'd expect compilation to fail at the spot where pthread_testcancel() is actually called in the code, since it has no arguments there. This is different from the stack-overflow discussion, which is not about macro definitions at all.

I'd suggest trying to build with this change to mosquitto_internal.h:

+++ lib/mosquitto_internal.h    2022-01-04 19:54:01.119053000 -0500
@@ -33,7 +33,7 @@
 #endif
 #include <stdlib.h>
 
-#if defined(WITH_THREADING) && !defined(WITH_BROKER)
+#if defined(WITH_THREADING)
 #  include <pthread.h>
 #else
 #  include <dummypthread.h>

which bypasses dummypthread outright, but might cause other problems because there's a lot of code that does things differently when BROKER is defined, and that might include some data structure initialisation.

[pfichtner]

Thanks for your explanation @ptjm but I don't understand where this C function is that doesn't take arguments: In both glibc and uClibc/uClibc-ng sources the function is defined as "extern void pthread_testcancel (void)".

You can make it stop failing at that point by definition the macro to take an argument, but then the macro definition won't match the C function definition, and I'd expect compilation to fail at the spot where pthread_testcancel()

Where is that C function definition of pthread_testcancel() without any argument matching those of dummythread.h's definition?

[ptjm]

"extern void pthread_testcancel (void);" is an ANSI C prototype for a function which doesn't take an argument. What we have in dummypthread is a C-preprocessor definition for a macro which doesn't take a function "#define pthread_testcancel()". In the actual code, we have a call to this function,without any arguments (this is from loop.c in version 2.0.14):

#ifdef HAVE_PTHREAD_CANCEL
                        pthread_testcancel();
#endif
                        rc = mosquitto_loop(mosq, timeout, max_packets);

The effect of the macro is to replace that call to pthread_testcancel() with nothing. Your problem is that pthread.h gets included through some of the system headers, but the preprocessor doesn't know the difference between a function call (no arguments) and a function prototype ("void" to indicate that the function takes no arguments), so it complains about the prototype. Your solution is to add an argument to the macro, and I was saying this isn't a good solution and will cause a problem when it gets to the code that actually calls the macro -- however I'm wrong, because the preprocessor doesn't know the difference between a macro call with one argument and a macro call with no arguments.

I still think it's not a good solution. If there's a need for pthread.h in the standard library, including dummypthread.h first might cause issues, since it makes a few core pthread functions no-ops. My preference in this case would be to either build the broker with the thread calls in place or to change the way the pthread functions are stubbed out, e.g., like this:

#if defined(WITH_THREADING)
# define PTHREAD_CREATE pthread_create
# define PTHREAD_JOIN pthread_join
# define PTHREAD_CANCEL pthread_cancel
# define PTHREAD_TESTCANCEL pthread_testcancel

# define PTHREAD_MUTEX_INIT pthread_mutex_INIT
# define PTHREAD_MUTEX_DESTROY pthread_mutex_DESTROY
# define PTHREAD_MUTEX_LOCK pthread_mutex_LOCK
# define PTHREAD_MUTEX_UNLOCK pthread_mutex_UNLOCK
#else
# define PTHREAD_CREATE(A, B, C, D)
# define PTHREAD_JOIN(A, B)
# define PTHREAD_CANCEL(A)
# define PTHREAD_TESTCANCEL()

# define PTHREAD_MUTEX_INIT(A, B)
# define PTHREAD_MUTEX_DESTROY(A)
# define PTHREAD_MUTEX_LOCK(A)
# define PTHREAD_MUTEX_UNLOCK(A)
#endif```
then use the upper-cased macro calls in the code.

[pfichtner]

Thanks @ptjm for your detailed and competent explanation and the time you took for it!

[ralight]

Thanks for the analysis @ptjm, I'm not sure I agree about your final point though. dummypthread.h is only included in the mosquitto code, so it only affects the mosquitto code being compiled. It doesn't stop the standard library using those functions if they need to.

I think there needs to be a separate review of the threading use in the broker, at the very least there should be some help for plugins that might want to use separate threads. At that point this will become moot.