QW-Group / mvdsv

MVDSV: a QuakeWorld server
GNU General Public License v2.0
58 stars 56 forks source link

Use proper CMAKE_DL_LIBS variable instead of hardcoded OS's #106

Closed brad0 closed 1 year ago

brad0 commented 1 year ago

This is cleaner and this applies to other *BSD OS's and possibly other OS's.

tcsabina commented 1 year ago

Hi @brad0,

This was recently (1 month ago) added by Tom via PR#101. I don't have a BSD system, so I cannot verify the impact of this. Are we sure that this is indeed a better/cleaner approach?

namtsui commented 1 year ago

I tested this and it works with mvdsv (and ktx) on openbsd. It should be cleaner because you don't have to specify all the operating systems that do not have dl. It looks like it passes the linux continuous integration.

Also, in openbsd ports there are some tips: https://www.openbsd.org/faq/ports/guide.html

Test for features, not for specific OSes.

_OpenBSD__ should be used sparingly, if at all. Constructs that look like
#if defined(__NetBSD__) || defined(__FreeBSD__)
are often inappropriate.
brad0 commented 1 year ago

I do, this is indeed the better / cleaner way of doing things.

tcsabina commented 1 year ago

Fair enough!

So we should have this in all our projects? KTX, qtv, mvdparser, qwdtools?

namtsui commented 1 year ago

So we should have this in all our projects? KTX, qtv, mvdparser, qwdtools?

I don't think it's needed because -ldl is used for dlopen() on linux. I grepped these projects and only found dlopen() calls in mvdsv.

$ ag dlopen 
mvdsv/src/sv_sys_win.c
689:DL_t Sys_DLOpen (const char *path)

mvdsv/src/sv_sys_unix.c
556:DL_t Sys_DLOpen(const char *path)
558: DL_t ret = dlopen(path,
566:         Con_DPrintf("Sys_DLOpen: %s\n", dlerror());

mvdsv/src/sys.h
149:DL_t Sys_DLOpen (const char *path);

mvdsv/src/vm.c
1205:                vm->dllHandle = Sys_DLOpen( name );
brad0 commented 1 year ago

Linux OS's based on glibc 2.34 and newer don't even need -ldl..

* In order to support smoother in-place-upgrades and to simplify
  the implementation of the runtime all functionality formerly
  implemented in the libraries libpthread, libdl, libutil, libanl has
  been integrated into libc.  New applications do not need to link with
  -lpthread, -ldl, -lutil, -lanl anymore.  For backwards compatibility,
  empty static archives libpthread.a, libdl.a, libutil.a, libanl.a are
  provided, so that the linker options keep working.  Applications which
  have been linked against glibc 2.33 or earlier continue to load the
  corresponding shared objects (which are now empty).  The integration
  of those libraries into libc means that additional symbols become
  available by default.  This can cause applications that contain weak
  references to take unexpected code paths that would only have been
  used in previous glibc versions when e.g. preloading libpthread.so.0,
  potentially exposing application bugs.
tcsabina commented 1 year ago

thanks guys for the 'research'!