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

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/include/signal.h.def:14-17
+#define __need_size_t
+#include <stddef.h>
 
 %%include_file(${platform_signal})
----------------
abrachet wrote:
> I need to get <linux/signal.h> included first now because the functions take `sigset_t *`. I couldn't figure out a clean way to do this but I'm pretty sure this isn't it.
If `linux.h` require `size_t`, the platform file should be constructed this way:

```
#define __need_size_t
#include <stddef.h>

#include <linux/signal.h>
```


================
Comment at: libc/spec/posix.td:1-5
+def SigSetType : NamedType<"sigset_t">;
+def SigSetPtrType : PtrType<SigSetType>;
+def ConstSigSetPtrType : ConstType<SigSetPtrType>;
+def RestrictSigSetType : RestrictedPtrType<SigSetType>;
+def ConstRestrictSigSetType : ConstType<RestrictSigSetType>;
----------------
abrachet wrote:
> Perhaps not where these should go?
If `sigset_t` is also defined and used by the C standard, then put it in `spec/spec.td`


================
Comment at: libc/test/src/signal/CMakeLists.txt:23-24
     signal_h
+    sigprocmask
+    __errno_location
+    signal_test
----------------
abrachet wrote:
> @sivachandra, I'm not sure how `add_header_library` is supposed to work, but I needed to add these to use `signal_test` even though it already depends on `sigprocmask` and `__errno_location`.
It shouldn't be required if CMake is setting up the deps correctly. Let me do some digging and get back to you.


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

https://reviews.llvm.org/D75026