[libc-commits] [PATCH] D74528: [libc] Add Initial Support for Signals

[Alex Brachet via Phabricator via libc-commits]

abrachet marked an inline comment as done.
abrachet added inline comments.


================
Comment at: libc/config/linux/api.td:144
+    // We use <linux/signal.h> so these should be defined.
+    MacroDefineIfNot<"NSIG", "64">,
+
----------------
MaskRay wrote:
> MaskRay wrote:
> > What is it 64?
> 65 may be a reasonable choice for most architectures.
> 
> MIPS may require 129. A MIPS signal mask contains 128 bits.
> 
> The Linux uapi may be stuck with `#define SIGRTMAX _NSIG` but a libc should not replicate that (mistake). A libc should have `SIGRTMAX-1 == _NSIG`, though `SIGRTMAX` is not necessarily a constant.
> 
Good catch.

My /usr/include/x86_64-linux-gnu/asm/signal.h has:
```
#define NSIG		32
typedef unsigned long sigset_t;
// ... further down
#define SIGRTMIN	32
#define SIGRTMAX	_NSIG
```
This is where I am currently getting `sigset_t` and NSIG. Should we not rely on this header then and define these our self? I do want to use this include to give me `sigset_t`, `struct sigaction`, because I think this lessens the burden on us. I would want to hear your thoughts on how difficult (or easy) it is to maintain portability between architectures without using the Linux headers.

> 65 may be a reasonable choice for most architectures.
>
> MIPS may require 129. A MIPS signal mask contains 128 bits.
We already have x86_64 specific directory, for header gen config here, it shouldn't be a big problem to make NSIG platform dependent, I'm happy to do it in this patch, if we step away from using <linux/signal.h>


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74528/new/

https://reviews.llvm.org/D74528