search

CESNET / netopeer2

NETCONF toolset
BSD 3-Clause "New" or "Revised" License
299 stars 188 forks source link

build: `pthread_rwlock_t` is not available without feature macros #1377

Closed jktjkt closed 1 year ago

jktjkt commented 1 year ago

When building src/netconf_monitoring.c with all the custom pthread functionality disabled via CMake options (i.e., orcing the usage of the compat library), the build fails with the following error:

Netopeer2/compat/compat.h:149:32: error: unknown type name 'pthread_rwlock_t'

I was getting the same error on my NixOS with clang 15.0.7, but also on an older Fedora with clang 12.

This exact same header file is included from a variety of other places, and (which is where these symbols live) is included as well. It turns out that the pthread_rwlock_t was added in Issue 5 of POSIX, and when using the C99 standard along with no feature test macros to pick a specific version of POSIX functions, these might not be visible. Indeed this place does not define _GNU_SOURCE (which brings in a lot of stuff, including a new enough POSIX).

Solve this by using _GNU_SOURCE also from this file.

Fixes: 7e5349b compat UPDATE use latest compat lib Fixes: 28138d5 build UPDATE use c99 standard instead of gnu99

michalvasko commented 1 year ago

Right, I have been fixing this in some other project not that long ago. But shouldn't then the macro be in compat.h? That is where the typedef is not found.

jktjkt commented 1 year ago

I tried that, but that's too late. By the time compat.h is included, which includes pthread.h, something else might have already included pthread.h directly, and therefore the new value of _POSIX_C_SOURCE has no effect because of the include guards.

IMHO a proper fix (without relying on the all the users of compat.h doing the right thing) would be something like a global add_definitions(_GNU_SOURCE) from within CMake. Or, alternatively, you can add an #if _POSIX_C_SOURCE < 20XXYYZZblahblahL #error this header needs extra feature test macros #endif or something like that. Your call, this is C and opinion based; please feel free to push whatever you find as the most beautiful approach :).

samuel-gauthier commented 1 year ago

Maybe you could also put std=gnu99 instead of c99 when those rwlocks are needed, in CMakeList.txt. This way, c99 is still enforced when we know that the rwlocks are not needed.

michalvasko commented 1 year ago

I have decided to put it into the header so that it can always be safely included but yeah, no solution seems perfect. I would prefer not to depend on cmake adding the required macros/standards which is why I have opted for this solution.