[libc-commits] [PATCH] D75802: [libc] Add sigaction

[Alex Brachet via Phabricator via libc-commits]

abrachet marked 4 inline comments as done.
abrachet added inline comments.


================
Comment at: libc/config/linux/signal.h.in:11
 
+#define sigaction __kernel_sigaction
 #include <linux/signal.h>
----------------
sivachandra wrote:
> abrachet wrote:
> > This is very not ideal, but the Linux headers expose a different `struct sigaction` than POSIX expects, it has no `sa_action` member.
> What do you think of using the other way round scheme. As in, in our public signal.h header, we will define `sigaction` like this:
> 
> ```
> struct __sigaction {
>   ...
> };
> 
> #ifndef __LLVM_LIBC_INTERNAL_SIGACTION
> typedef __sigaction sigaction;
> #endif
> ```
> 
> This will allow us to include our public header in internal code as:
> 
> ```
> #define __LLVM_LIBC_INTERNAL_SIGACTION
> #include "include/signal.h"
> ```
> 
> Throughout the internal implementation code, we will use `struct __sigaction` instead of `struct sigaction`. This way, we don't have to mess with the kernel headers. Does this make sense?
How does this look?


================
Comment at: libc/src/signal/linux/CMakeLists.txt:15
 
+add_custom_target(__restore
+  COMMAND ${CMAKE_CXX_COMPILER} ${CMAKE_CURRENT_SOURCE_DIR}/__restore.cpp -fomit-frame-pointer
----------------
sivachandra wrote:
> I have a similar requirement for a change I have been working on. I will share more on this soon. I am OK with this setup as is, but I have a feeling we can make use of similar items elsewhere as well.
I don't quite love how this is currently so I'd be happy to wait on that patch. Especially because I also end up touching cmake/modules/LLVMLibcRules.cmake in this patch which is not ideal. The only thing I can think of which depends on this patch is implementing `signal` and fixing abort, which I'm fine waiting on.


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

https://reviews.llvm.org/D75802