[libc-commits] [PATCH] D75026: [libc] Add sigprocmask

[Alex Brachet via Phabricator via libc-commits]

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


================
Comment at: libc/src/signal/linux/sigaddset.cpp:19
+int LLVM_LIBC_ENTRYPOINT(sigaddset)(sigset_t *set, int signum) {
+  if (!set || (unsigned)signum > (8 * sizeof(sigset_t))) {
+    llvmlibc_errno = EINVAL;
----------------
MaskRay wrote:
> abrachet wrote:
> > MaskRay wrote:
> > > `_NSIG` is expected to be compared.
> > `_NSIG`'s value has still not been decided, so I didn't want to use it. Should I just go ahead and define it as 65?
> This is a good example of layering issue. `_NSIG` is not decided yet (for no good reasons. I think we don't care about MIPS, at least for a forseeable future) => so we don't use it in this patch => this patch gets approved => it is committed.
> 
> Someday we add `_NSIG` => the patch needs rework.
> 
> (Well, doing things this way can cause a large number of commits. If someone judges the contribution by just counting the number of commits....)
`NSIG` was just not decided previously. It's not about number of commits, I doubt I would even be the one adding support for MIPS anyway. We just didn't come to a conclusion on last patch. I'm happy to decide it now. If you're happy with 65 (or should I just #define NSIG _NSIG [[ https://github.com/torvalds/linux/blob/master/arch/x86/include/asm/signal.h#L11 | which is 64 ]]?)  then I will go ahead and add that and use that instead of `8 * sizeof(sigset_t)`.
The same for `struct Sigset` I will just use `sigset_t` directly if you think that is better.


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

https://reviews.llvm.org/D75026