-Wcast-function-type warnings

[Sackzement]

I was wondering, if there is a way to fix these warnings:

/path/to/SDL-git/src/events/SDL_quit.c: In function ‘SDL_EventSignal_Init’:
/path/to/SDL-git/src/events/SDL_quit.c:78:41: warning: cast between incompatible function types from ‘void (*)(int,  siginfo_t *, void *)’ to ‘void (*)(int)’ [-Wcast-function-type]
   78 |     if (action.sa_handler == SIG_DFL && (void (*)(int))action.sa_sigaction == SIG_DFL) {
      |                                         ^

/path/to/SDL-git/src/core/linux/SDL_evdev_kbd.c: In function ‘kbd_register_emerg_cleanup’:
/path/to/SDL-git/src/core/linux/SDL_evdev_kbd.c:292:96: warning: cast between incompatible function types from ‘void (*)(int,  siginfo_t *, void *)’ to ‘void (*)(int)’ [-Wcast-function-type]
  292 |         if ((signum == SIGHUP || signum == SIGPIPE) && (old_action_p->sa_handler != SIG_DFL || (void (*)(int))old_action_p->sa_sigaction != SIG_DFL)) {
      |                                                                                                ^

Edit: I was testing compilations with the flag -Wextra enabled. But I'm not sure if it is worth fixing these. Here are some warnings that popped up:

-Wcast-function-type // 2x
-Wmissing-field-initializers // 4x
-Wsign-compare // a lot of these
-Wunused-parameter // a lot of these

Edit2: All the above warnings show up, when compiling with gcc. When compiling with clang, the two -Wcast-function-type don't show up, but all the other ones do.

[slouken]

Feel free to create a PR to fix these. I think you might be able to remove the signal handler casts.

[slouken]

Oh, don't bother fixing -Wunused-parameter. I'm not sure about sign compare. I tend to fix them when they come up, but the fix is usually a cast and it's not always obvious what the correct cast is.

[Akaricchi]

Be careful with casting function pointers, it can lead to problems on some platforms (e.g. WASM). Especially if the function is being called through a pointer with a different signature than it was declared with.

[smcv]

don't bother fixing -Wunused-parameter

In many of the projects I maintain, the build system explicitly turns this one off. It has a poor signal to noise ratio in any API that involves callbacks.

-Wcast-function-type

In places where more force is needed, gcc will let you cast any function pointer to/from a void (*) (void) without this warning, which makes it into the equivalent of void * for data pointers:

 typedef void (*SigactionFunc) (int, siginfo_t *, void *);

-foo = (SigactionFunc) bar;
+foo = (SigactionFunc) (void (*) (void)) bar;

In GLib-world, this type is typedef'd as GCallback. In SDL it's the same as __GLXextFuncPtr and __eglMustCastToProperFunctionPointerType but it might be worthwhile to have a more self-documenting typedef like

typedef void (*SDLFunction) (void);

...

foo = (SigactionFunc) (SDLFunction) bar;

[smcv]

Be careful with casting function pointers

Yes, this has a warning for a reason. It can be done, but only if there is an out-of-band way to keep track of the function's real type - for example in sigaction(), the sa_handler and sa_sigaction members of struct sigaction are sometimes the same bytes of memory (via a union), and the out-of-band tracking for which signature is the right one is based on whether sa_flags contains SA_SIGINFO.

if the function is being called through a pointer with a different signature than it was declared with

In general this is undefined behaviour, although I think it might be allowed if the signatures only have trivial differences such as adding/removing const.

[smcv]

-Wmissing-field-initializers

This is another warning with a poor signal-to-noise ratio and I'd suggest explicitly turning it off. The effect of a missing field initializer is well-defined in the C standard (pointers become NULL, numbers become zero), and some APIs involve structs that were specifically designed to be partially-initialized like this.

[slouken]

SDL already has SDL_FunctionPointer, which can probably be used to cast in this case.

[Sackzement]

I found the flags -Wcast-function-type and -Wcast-function-type-strict for clang. These weren't automatically enabled with -Wextra. These flags detect many many more unregular function casts.

Here are some interesting stats about how many were found in which files: I separated them into categories.

// signals:
src/events/SDL_quit.c                       1 warning  generated.
src/core/linux/SDL_evdev_kbd.c              1 warning  generated.

// misc:
src/hidapi/SDL_hidapi.c                    29 warnings generated.
src/core/linux/SDL_dbus.c                  42 warnings generated.
src/storage/steam/SDL_steamstorage.c        9 warnings generated.

// graphics:
src/render/opengl/SDL_render_gl.c          54 warnings generated.
src/render/opengl/SDL_shaders_gl.c         15 warnings generated.
src/render/opengles2/SDL_render_gles2.c    56 warnings generated.
src/render/vulkan/SDL_render_vulkan.c      96 warnings generated.
src/video/SDL_egl.c                        29 warnings generated.
src/video/SDL_video.c                       9 warnings generated.
src/video/SDL_vulkan_utils.c                8 warnings generated.
src/video/x11/SDL_x11opengl.c              16 warnings generated.
src/video/x11/SDL_x11vulkan.c              10 warnings generated.
src/video/kmsdrm/SDL_kmsdrmvulkan.c        13 warnings generated.
src/video/wayland/SDL_waylandvulkan.c       7 warnings generated.
src/video/offscreen/SDL_offscreenvulkan.c   7 warnings generated.
src/gpu/vulkan/SDL_gpu_vulkan.c           114 warnings generated.

// tests:
src/test/SDL_test_harness.c                 1 warning  generated.
testffmpeg.c                                2 warnings generated.
testffmpeg_vulkan.c                        28 warnings generated.
testgl.c                                   46 warnings generated.
testgles2.c                                56 warnings generated.
testgles2_sdf.c                            56 warnings generated.
testloadso.c                                1 warning  generated.
testshader.c                               12 warnings generated.
testvulkan.c                               44 warnings generated.