`SDL_ELF_NOTE_DLOPEN` cannot be used with a toolchain not supporting variadic macros

[madebr]

SDL_ELF_NOTE_DLOPEN of SDL_dlopennote.h is currently designed to be used unconditionally.

https://github.com/libsdl-org/SDL/blob/45d65f6e1f51422c414e85f133849648918a8c09/include/SDL3/SDL_dlopennote.h#L102-L109

However, it cannot be used with an (older) toolchain not supporting variadic macros.

Compiling testdlopennote.c of this PR fails with the error below (when using MSVC 6):

testdlopennote.c(13) : error C2059: syntax error : 'string'

Defining SDL_DISABLE_DLOPEN_NOTES does not help since it is already defined (Windows is not a unix platform).

[slouken]

I thought @sezero had a suggestion on how to fix this?

[sezero]

For old gcc, I propose the following: disables funtionality for gcc < 3.1, tested with gcc2.95.

Don't know what to do with old MSVC.

diff --git a/include/SDL3/SDL_dlopennote.h b/include/SDL3/SDL_dlopennote.h
index b4e6088..651ce69 100644
--- a/include/SDL3/SDL_dlopennote.h
+++ b/include/SDL3/SDL_dlopennote.h
@@ -104,6 +104,11 @@
 #ifndef SDL_DISABLE_DLOPEN_NOTES
 #define SDL_DISABLE_DLOPEN_NOTES
 #endif
+#elif defined(__GNUC__) && (__GNUC__ < 3 || (__GNUC__ == 3 && __GNUC_MINOR__ < 1))
+/* gcc < 3.1 too old */
+#ifndef SDL_DISABLE_DLOPEN_NOTES
+#define SDL_DISABLE_DLOPEN_NOTES
+#endif
 #endif /* SDL_PLATFORM_UNIX || SDL_PLATFORM_ANDROID */
 
 #if defined(__ELF__) && !defined(SDL_DISABLE_DLOPEN_NOTES)
@@ -204,7 +209,12 @@
         "\",\"soname\":" SDL_SONAME_ARRAY(__VA_ARGS__) "}]",     \
         SDL_ELF_NOTE_UNIQUE_NAME)
 
-#elif (defined (__GNUC__) && __GNUC__ < 3) || (defined(_MSC_VER) && (_MSC_VER < 1400))
+#elif defined(__GNUC__) && (__GNUC__ < 3)
+
+/* Variadic macros are not supported */
+#define SDL_ELF_NOTE_DLOPEN(args...)
+
+#elif defined(_MSC_VER) && (_MSC_VER < 1400)
 
 /* Variadic macros are not supported */
 #define SDL_ELF_NOTE_DLOPEN

[sezero]

Pushed the above patch with minor revisions as https://github.com/libsdl-org/SDL/commit/04aa74b5f93a965c34e94728d2d3a64757899061

Ancient MSVC issues still not resolved.

[slouken]

+/* Variadic macros are not supported */ +#define SDL_ELF_NOTE_DLOPEN(args...) +

Doesn't this syntax mean variadic macros are supported?

[sezero]

+/* Variadic macros are not supported */ +#define SDL_ELF_NOTE_DLOPEN(args...) +

Doesn't this syntax mean variadic macros are supported?

It does, so I dropped that note when committing :)

[madebr]

Pushed the above patch with minor revisions as 04aa74b

Thanks! I can build testdlopennote.c with gcc 2.95.4 from the debian/eol:woody docker container.

[madebr]

I'm currently experimenting with turning SDL_ELF_NOTE_DLOPEN in a non-variadic macro like so:

#define SDL_ELF_NOTE_DLOPEN(feature, description, priority, libs)       \
    SDL_ELF_NOTE_INTERNAL(                                              \
        "[{\"feature\":\"" feature                                      \
        "\",\"description\":\"" description                             \
        "\",\"priority\":\"" priority                                   \
        "\",\"soname\":" SDL_ELF_NOTE_JOIN(SDL_SONAME_ARRAY,libs) "}]", \
        SDL_ELF_NOTE_UNIQUE_NAME)

and require usage as:

SDL_ELF_NOTE_DLOPEN(
    "png",
    "Support for loading PNG images using libpng",
    SDL_ELF_NOTE_DLOPEN_PRIORITY_RECOMMENDED,
    ("libpng16.so.16")  /* libraries should be encloses with braces */
);

Preprocessing this with -E -P (using modern gcc 15.2) results in a correct expanded macro, but compiling the source fails with the following error:

In file included from /home/maarten/projects/SDL/include/SDL3/SDL.h:45,
                 from test/testdlopennote.c:1:
/home/maarten/projects/SDL/include/SDL3/SDL_dlopennote.h:209:44: error: pasting "SDL_SONAME_ARRAY" and "(" does not give a valid preprocessing token
  209 |         "\",\"soname\":" SDL_ELF_NOTE_JOIN(SDL_SONAME_ARRAY,libs) "}]", \
      |                                            ^~~~~~~~~~~~~~~~
/home/maarten/projects/SDL/include/SDL3/SDL_dlopennote.h:168:33: note: in definition of macro ‘SDL_ELF_NOTE_JOIN2’
  168 | #define SDL_ELF_NOTE_JOIN2(A,B) A##B
      |                                 ^
/home/maarten/projects/SDL/include/SDL3/SDL_dlopennote.h:209:26: note: in expansion of macro ‘SDL_ELF_NOTE_JOIN’
  209 |         "\",\"soname\":" SDL_ELF_NOTE_JOIN(SDL_SONAME_ARRAY,libs) "}]", \
      |                          ^~~~~~~~~~~~~~~~~
test/testdlopennote.c:12:1: note: in expansion of macro ‘SDL_ELF_NOTE_DLOPEN’
   12 | SDL_ELF_NOTE_DLOPEN(
      | ^~~~~~~~~~~~~~~~~~~

Do you have suggestions?

[madebr]

If we don't find a way, I would remove the following lines from SDL_dlopennote.h: https://github.com/libsdl-org/SDL/blob/281ac6c3bb619f614616a82891c3e39d3f05ceea/include/SDL3/SDL_dlopennote.h#L218-L219

Projects can still use the SDL_ELF_NOTE_DLOPEN freely.

It's only when they want to provide compatibility with older toolchains that they need to protect its use with #ifdef SDL_ELF_NOTE_DLOPEN.

[slouken]

If we don't find a way, I would remove the following lines from SDL_dlopennote.h:

SDL/include/SDL3/SDL_dlopennote.h

Lines 218 to 219 in 281ac6c

/* Variadic macros are not supported */ #define SDL_ELF_NOTE_DLOPEN Projects can still use the SDL_ELF_NOTE_DLOPEN freely.

It's only when they want to provide compatibility with older toolchains that they need to protect its use with #ifdef SDL_ELF_NOTE_DLOPEN.

This seems like a good idea. Want to make that change?

[sezero]

If we don't find a way, I would remove the following lines from SDL_dlopennote.h:

SDL/include/SDL3/SDL_dlopennote.h

Lines 218 to 219 in 281ac6c /* Variadic macros are not supported */ #define SDL_ELF_NOTE_DLOPEN

I think removing that broken line is right thing to do. If a solution is found for an unhandled case that can be added later.

[madebr]

I'll remove the line(s), can you first check whether you can get this suggestion working? If if's viable, then we can avoid a build breaking change.

[sezero]

I'll remove the line(s), can you first check whether you can get this suggestion working? If if's viable, then we can avoid a build breaking change.

Can you attach a patch? (yeah, I'm lazy)

[madebr]

Can you attach a patch? (yeah, I'm lazy)

Sure, but I think simply copy/pasting that code in SDL3/SDL_dlopennote.h is easier :)

--- a/include/SDL3/SDL_dlopennote.h
+++ b/include/SDL3/SDL_dlopennote.h
@@ -201,12 +201,12 @@
  * \sa SDL_ELF_NOTE_DLOPEN_PRIORITY_RECOMMENDED
  * \sa SDL_ELF_NOTE_DLOPEN_PRIORITY_REQUIRED
  */
-#define SDL_ELF_NOTE_DLOPEN(feature, description, priority, ...) \
-    SDL_ELF_NOTE_INTERNAL(                                       \
-        "[{\"feature\":\"" feature                               \
-        "\",\"description\":\"" description                      \
-        "\",\"priority\":\"" priority                            \
-        "\",\"soname\":" SDL_SONAME_ARRAY(__VA_ARGS__) "}]",     \
+#define SDL_ELF_NOTE_DLOPEN(feature, description, priority, libs)       \
+    SDL_ELF_NOTE_INTERNAL(                                              \
+        "[{\"feature\":\"" feature                                      \
+        "\",\"description\":\"" description                             \
+        "\",\"priority\":\"" priority                                   \
+        "\",\"soname\":" SDL_ELF_NOTE_JOIN(SDL_SONAME_ARRAY,libs) "}]", \
         SDL_ELF_NOTE_UNIQUE_NAME)
 
 #elif defined(__GNUC__) && __GNUC__ < 3

[madebr]

Ok, the patch is little bit more complicated when you can remove the exceptions for older gcc etc:

diff --git a/include/SDL3/SDL_dlopennote.h b/include/SDL3/SDL_dlopennote.h
index da145eed5..9c8c3e207 100644
--- a/include/SDL3/SDL_dlopennote.h
+++ b/include/SDL3/SDL_dlopennote.h
@@ -201,26 +201,17 @@
  * \sa SDL_ELF_NOTE_DLOPEN_PRIORITY_RECOMMENDED
  * \sa SDL_ELF_NOTE_DLOPEN_PRIORITY_REQUIRED
  */
-#define SDL_ELF_NOTE_DLOPEN(feature, description, priority, ...) \
-    SDL_ELF_NOTE_INTERNAL(                                       \
-        "[{\"feature\":\"" feature                               \
-        "\",\"description\":\"" description                      \
-        "\",\"priority\":\"" priority                            \
-        "\",\"soname\":" SDL_SONAME_ARRAY(__VA_ARGS__) "}]",     \
+#define SDL_ELF_NOTE_DLOPEN(feature, description, priority, libs)       \
+    SDL_ELF_NOTE_INTERNAL(                                              \
+        "[{\"feature\":\"" feature                                      \
+        "\",\"description\":\"" description                             \
+        "\",\"priority\":\"" priority                                   \
+        "\",\"soname\":" SDL_ELF_NOTE_JOIN(SDL_SONAME_ARRAY,libs) "}]", \
         SDL_ELF_NOTE_UNIQUE_NAME)

-#elif defined(__GNUC__) && __GNUC__ < 3
-
-#define SDL_ELF_NOTE_DLOPEN(args...)
-
-#elif defined(_MSC_VER) && _MSC_VER < 1400
-
-/* Variadic macros are not supported */
-#define SDL_ELF_NOTE_DLOPEN
-
 #else

-#define SDL_ELF_NOTE_DLOPEN(...)
+#define SDL_ELF_NOTE_DLOPEN(feature, description, priority, libs)

 #endif

~It works fine with MSVC btw, but that does not matter since WIndows is not an ELF platform...~

[madebr]

I'm currently on Windows and did not realize the patch was incomplete for Linux: patch.txt

[sezero]

Patch ran through iconv -f UTF-16 -t UTF-8: patch.txt

Reproduced your experience: -E outputs are the same but patched version emits that preprocessor error and refuses to build.

I don't have a solution, at least not yet. @slouken?

[madebr]

Another alternative could be to require the last argument to be a C string, representing a javascript JSON list: e.g.

SDL_ELF_NOTE_DLOPEN(
    "png",
    "Support for loading PNG images using libpng",
    SDL_ELF_NOTE_DLOPEN_PRIORITY_RECOMMENDED,
    "[\"" LIBRARY1 "\",\"" LIBRARY2 "\"]"
);

(I don't like this suggestion)

[sezero]

(I don't like this suggestion)

Heh, me either.

Requiring working variadic macros functionality in SDL3 isn't a bad thing though: And it is for elf targets where we have compilers with that functionality for more than 20 years. I do suggest removing that broken define and be done with it.

[sezero]

One thing that can be done is, not including SDL_dlopennote.h in SDL.h by default and manually include it where it is actually used. That would prevent most accidents.

[madebr]

The macro is currently called SDL_ELF_NOTE_DLOPEN, but the header is called SDL_dlopennote.h. Do we want to keep the ELF in the macro?

[sezero]

I'd say just keep the macro name as is.

[madebr]

One thing that can be done is, not including SDL_dlopennote.h in SDL.h by default and manually include it where it is actually used. That would prevent most accidents.

You then need to know before including the header whether it is safe to do so.

I've asked in the C channel on the discord about this and got another suggestion:

// macro's
#define LIB(S) "\"" S "\""
#define AND_LIB(S) ",\"" S "\""
#define DLNOTE(LIBS) static const char *str = "[" LIBS "]"

// usage
DLNOTE(LIB("libSDL-1.2.so.0") AND_LIB("libSDL2-2.0.so.0") AND_LIB("libSDL3.so.0"));

It's still weird, but more manageable.

I'm still in favor of the variadic macro.