[15/17] msvc: do not pretend to support all signals
diff mbox series

Message ID 1491ea4140518d76d4619fa2da7ec20250da1404.1560860634.git.gitgitgadget@gmail.com
State New
Headers show
Series
  • Fix MSVC support, at long last
Related show

Commit Message

Marco Trevisan (TreviƱo) via GitGitGadget June 18, 2019, 12:24 p.m. UTC
From: Jeff Hostetler <jeffhost@microsoft.com>

This special-cases various signals that are not supported on Windows,
such as SIGPIPE. These cause the UCRT to throw asserts (at least in
debug mode).

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/mingw.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

Comments

Eric Sunshine June 19, 2019, 4:10 a.m. UTC | #1
On Tue, Jun 18, 2019 at 8:24 AM Jeff Hostetler via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> This special-cases various signals that are not supported on Windows,
> such as SIGPIPE. These cause the UCRT to throw asserts (at least in
> debug mode).
>
> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> diff --git a/compat/mingw.c b/compat/mingw.c
> @@ -2119,8 +2119,34 @@ int mingw_raise(int sig)
> +#if defined(_MSC_VER)
> +               /*
> +                * <signal.h> in the CRT defines 8 signals as being
> +                * supported on the platform.  Anything else causes
> +                * an "Invalid signal or error" (which in DEBUG builds
> +                * causes the Abort/Retry/Ignore dialog).  We by-pass
> +                * the CRT for things we already know will fail.
> +                */
> +               /*case SIGINT:*/
> +       case SIGILL:

Why is SIGINT commented out?

And, the comment block seems over-indented.

> +       case SIGFPE:
> +       case SIGSEGV:
> +       case SIGTERM:
> +       case SIGBREAK:
> +       case SIGABRT:
> +       case SIGABRT_COMPAT:
> +               return raise(sig);
> +       default:
> +               errno = EINVAL;
> +               return -1;
Johannes Schindelin June 19, 2019, 4:49 p.m. UTC | #2
Hi Eric,

On Wed, 19 Jun 2019, Eric Sunshine wrote:

> On Tue, Jun 18, 2019 at 8:24 AM Jeff Hostetler via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> > This special-cases various signals that are not supported on Windows,
> > such as SIGPIPE. These cause the UCRT to throw asserts (at least in
> > debug mode).
> >
> > Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > diff --git a/compat/mingw.c b/compat/mingw.c
> > @@ -2119,8 +2119,34 @@ int mingw_raise(int sig)
> > +#if defined(_MSC_VER)
> > +               /*
> > +                * <signal.h> in the CRT defines 8 signals as being
> > +                * supported on the platform.  Anything else causes
> > +                * an "Invalid signal or error" (which in DEBUG builds
> > +                * causes the Abort/Retry/Ignore dialog).  We by-pass
> > +                * the CRT for things we already know will fail.
> > +                */
> > +               /*case SIGINT:*/
> > +       case SIGILL:
>
> Why is SIGINT commented out?

Whoops. The `case` before that already handles `SIGINT`, I think that's
why... I removed it.

> And, the comment block seems over-indented.

Not really, as the `case` statements are indented one level less than the
code (including the comments).

But I agree that it looks funny, and moved it within the `case` arm.

Thanks!
Dscho

>
> > +       case SIGFPE:
> > +       case SIGSEGV:
> > +       case SIGTERM:
> > +       case SIGBREAK:
> > +       case SIGABRT:
> > +       case SIGABRT_COMPAT:
> > +               return raise(sig);
> > +       default:
> > +               errno = EINVAL;
> > +               return -1;
>

Patch
diff mbox series

diff --git a/compat/mingw.c b/compat/mingw.c
index 667285887a..8b56aa5773 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2119,8 +2119,34 @@  int mingw_raise(int sig)
 			sigint_fn(SIGINT);
 		return 0;
 
+#if defined(_MSC_VER)
+		/*
+		 * <signal.h> in the CRT defines 8 signals as being
+		 * supported on the platform.  Anything else causes
+		 * an "Invalid signal or error" (which in DEBUG builds
+		 * causes the Abort/Retry/Ignore dialog).  We by-pass
+		 * the CRT for things we already know will fail.
+		 */
+		/*case SIGINT:*/
+	case SIGILL:
+	case SIGFPE:
+	case SIGSEGV:
+	case SIGTERM:
+	case SIGBREAK:
+	case SIGABRT:
+	case SIGABRT_COMPAT:
+		return raise(sig);
+	default:
+		errno = EINVAL;
+		return -1;
+
+#else
+
 	default:
 		return raise(sig);
+
+#endif
+
 	}
 }