cameron314 added inline comments.
================
Comment at: tools/driver/Driver.cpp:1314
@@ -1313,1 +1313,3 @@
+ signal(SIGINT, sigint_handler);
+#ifndef _MSC_VER
signal (SIGPIPE, SIG_IGN);
----------------
zturner wrote:
> I think `_MSC_VER` is the right check, because the builtin `signal`
> implementation is something that is part of Visual Studio. If you were using
> Cygwin I imagine some of those constants might be present (although tbh, I
> don't ever use Cygwin and I don't know if anyone else does either, so someone
> would need to confirm).
>
> `_MSC_VER` is also defined for clang-cl, so this check would catch MSVC and
> clang-cl
I was thinking more of mingw-w64, which tries very hard to have a compatible
set of headers as those defined in the Windows SDK (and does not define
`_MSC_VER`). But many, many other places in LLDB already check `_MSC_VER`
anyway.
================
Comment at: tools/driver/Platform.cpp:93-97
@@ -92,7 @@
- {
- case ( SIGINT ):
- {
- _ctrlHandler = sigFunc;
- SetConsoleCtrlHandler( CtrlHandler, TRUE );
- }
- break;
----------------
zturner wrote:
> Yes, because now `MIDriverMain.cpp` should end up calling the builtin version
> of `signal()`, which also calls `SetConsoleCtrlHandler`. As far as I can
> tell there's no actual difference (technically, the builtin version is a
> strict superset of this version, so if anything it should be better)
Great!
================
Comment at: tools/driver/Platform.h:40
@@ -39,14 +39,3 @@
-
- // signal handler function pointer type
- typedef void(*sighandler_t)(int);
-
- // signal.h
- #define SIGINT 2
- // default handler
- #define SIG_DFL ( (sighandler_t) -1 )
- // ignored
- #define SIG_IGN ( (sighandler_t) -2 )
-
// signal.h
#define SIGPIPE 13
----------------
amccarth wrote:
> Now that we are including <signal.h>, won't the following defines create
> duplicate definition warnings for the non-Windows platforms?
It's only included on Windows, though -- unless some other TU includes both
this header and `signal.h`?
http://reviews.llvm.org/D18287
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
