mattgodbolt / seasocks

Simple, small, C++ embeddable webserver with WebSockets support
BSD 2-Clause "Simplified" License
734 stars 120 forks source link

gettid glibc 2.30 #138

Closed ColonelPanic42 closed 4 years ago

ColonelPanic42 commented 4 years ago

Until glibc 2.29 there was no implementation of the gettid function for getting the current thread ID. Therefore a common workaround was implemented in seasocks/Server.cpp line 98ff.

Beginning with glibc 2.30 the missing function was included in unistd_ext.h.

Therefore I would suggest doing something like the following in seasocks/Server.cpp

#define GLIBC_2_30 1
...
#if (!GLIBC_2_30)
    pid_t gettid() {
        return static_cast<pid_t>(syscall(SYS_gettid));
    }
#endif
...
offa commented 4 years ago

This sounds reasonable to me.

Maybe we can replace it's usage with std::thread::get_id() eventually.

offa commented 4 years ago

Related to #129.

offa commented 4 years ago

@ColonelPanic42 I have opened PR #139.

rspielmann commented 3 years ago

Hello :)

I stumbled across this change yesterday after getting pretty sad output like this while trying to build seasocks with a cross toolchain:

[ 29%] Building CXX object src/main/c/CMakeFiles/seasocks.dir/Server.cpp.o
/path/to/things/build/seasocks/src/seasocks/src/main/c/Server.cpp:100:19: error: missing binary operator before token "("
  100 | #if __GLIBC_PREREQ(2, 30)

__GLIBC_PREREQ is not necessarily always defined, for example when there is no glibc because you are on Alpine with musl :) I deduced that it would have been beneficial to check whether __GLIBC_PREREQ is defined before using it, and keeping the fallback otherwise. Would it make sense from your point of view to add such a check?

Thinking a bit more about this, I also asked myself why you don't use std::thread in seasocks. Is it for some reason better to combine find_package(Threads) and direct glibc interaction?

Kind regards, Robert