[libc-commits] [libc] [libc][signal] clean up usage of sighandler_t (PR #125745)

[via libc-commits]

================
@@ -10,5 +10,6 @@
 #define LLVM_LIBC_TYPES___SIGHANDLER_T_H
 
 typedef void (*__sighandler_t)(int);
----------------
enh-google wrote:

> Perhaps it would be better for me to `#include <linux/signal.h>` from our generated `signal.h` in order to provide `__sighandler_t` (and everything else that entails) such that users don't need to directly+explicitly depend on `<linux/signal.h>`.

yeah, i think i'm not alone in saying that user code generally shouldn't `#include <linux/*>`, and _especially_ not when there's a standard header.

i _think_ that's why musl and glibc don't seem to care as much as me about whether or not that even works --- they have a stronger "don't do that then" stance.

> It's worth deeper discussions with all of us about policy relating to kernel headers. (https://sourceware.org/glibc/wiki/Synchronizing_Headers comes to mind, but perhaps isn't the only approach as you've mentioned).

yeah, that's the "doesn't really work well, even for glibc" solution i've mentioned before. i do think it would be good to have a cleaner option, and maybe that's more likely now between your kernel connections _and_ there now [maybe] being two libcs that want to not duplicate anything that's in a uapi header _and_ the existing glibc "it would be _nice_ if you could include the glibc header and the uapi header without conflicts".

> But for now, I'm in a sprint to get llvm-libc's test harness to cross compile. The fix is to clean up the usage of `sighandler_t`, which is the intent of this PR.

yeah, don't mind me ... i'm just trying to brain dump as much of the bionic history as i can, in particular when we have things that went well vs things that went badly (and orthogonally things we did on purpose vs things that were accidents).

TL;DR: i do think `sighandler_t` is somewhat useful for source compatibility (and even "not in POSIX? not coming in!" musl has it, which is always useful anecdata in that it probably means something fairly significant was broken without it). i have no reason to believe `__sighandler_t` is useful, but bionic's default "never duplicate something that's in a uapi header" policy means we'd have to go out of our way to _not_ expose it, and i've never had any reason to believe _that_ was useful either. (and your yaml format doesn't work well for examples like signal() :-P )

https://github.com/llvm/llvm-project/pull/125745