Re: [PATCH 1/3] modules: add sd-dlopen module

Thanks for looking into this. Some comments on the first patch:

On 2026-07-17 17:11, [email protected] wrote:

> diff --git a/lib/sd-dlopen.h b/lib/sd-dlopen.h
> new file mode 100644

I see that this file differs slightly from the latest commit 
bb3fcd73ce13eb796fe72da4202422e105f7b711 for systemd on GitHub. I assume 
that the intent is to keep the copies identical, so that what's in 
Gnulib will exactly match what's in the systemd sources. To that end, 
the patch should contain changes to config/srclist.txt and 
config/srclistvars.sh so that updates in the systemd sources are 
communicated automatically to Gnulib. I can help doing that if it's not 
obvious.

> +#include <stdint.h>

There seems to be no real need for sd-dlopen.h to include <stdint.h>. 
The only use of stdint.h concepts I see is in "#define 
SD_ELF_NOTE_DLOPEN_TYPE UINT32_C(0x407c0c0a)". But this can be rewritten 
as "#define SD_ELF_NOTE_DLOPEN_TYPE 0x407c0c0aU", which has the same 
meaning on all platforms that will use SD_ELF_NOTE_DLOPEN_TYPE. It would 
be better for sd-dlopen.h to not include <stdint.h>, so that includers 
of sd-dlopen.h have a cleaner namespace, and so that the Gnulib module 
need not depend on the stdint-h module.

Similarly, tests/test-sd-dlopen.c need not use UINT32_C.


> +#if defined __ELF__ && (defined __GNUC__ || defined __clang__) \
> +        && defined HAVE_ASM_IFNDEF_ENDIF_BALIGN && HAVE_ASM_IFNDEF_ENDIF_BALIGN

This checks HAVE_ASM_IFNDEF_ENDIF_BALIGN twice; once should suffice. Or 
did you mean to check HAVE_DLOPEN instead?

> +          [[void *handle = dlopen (0, RTLD_LAZY);
jj...
> +            [[void *handle = dlopen (0, RTLD_LAZY);

This should use NULL rather than 0, as compilers are starting to 
complain about 0 in pointer contexts.

> +License:
> +LGPLv2+

This module is a little unusual in that the main source code is MIT-0 
rather than LGPLv2+. Although it's not incorrect to mark it as LGPLv2+ 
here since MIT-0 is compatible, perhaps we should instead add support 
for a new license category 'MIT-0'? (Though that'd be some work.)


> +#ifdef _SD_ELF_NOTE_DLOPEN
> +# error "sd-dlopen implementation enabled without assembler support"
> +#endif

This is in a test program. Is _SD_ELF_NOTE_DLOPEN intended to be a 
public symbol, visible to test programs? If not, perhaps it's better to 
not use it in test programs, which normally should rely only on the 
public interfaces.

What are the test programs testing for? Merely that the macros compile? 
Would it be better for them to also test that the ELF notes are actually 
inserted into the executable?


Also, is this module intended to be used only within Gnulib, as a 
modules that other Gnulib modules depend on? Or can it be used directly 
by an application? For example, can coreutils use it for how GNU 'sort' 
uses dlopen?