Re: [PATCH] mingw: make mingw_signal return the correct handler

2013-06-10 Thread Erik Faye-Lund
On Mon, Jun 10, 2013 at 7:48 AM, Johannes Sixt j.s...@viscovery.net wrote:
 From: Erik Faye-Lund kusmab...@gmail.com

 Returning the SIGALRM handler for SIGINT is not very useful.

 Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
 Signed-off-by: Johannes Sixt j...@kdbg.org
 ---
 Am 6/7/2013 16:20, schrieb Erik Faye-Lund:
 On Fri, Jun 7, 2013 at 3:07 PM, Johannes Sixt j.s...@viscovery.net wrote:
 BTW, isn't mingw_signal() bogus in that it returns the SIGALRM handler
 even if a SIGINT handler is installed?

 Yep, that's a bug. Thanks for noticing.

 This is your patch to address it.

 I've pushed out a branch here that tries to address these issues, but
 I haven't had time to test them. I'll post the series when I have. In
 the mean time:

 https://github.com/kusma/git/tree/win32-signal-raise

 Concerning the other two patches:

 * SIGINT: perhaps handle only the SIG_DFL case (for the exit code) and
 forward all other cases to MSVCRT?

Perhaps. I'll have to think a bit about it, but it might very well be
the sanest approach, as long as it doesn't break git_terminal_prompt
(which was the reason for the change in the first place). I can't of
the top of my head understand why it should, though.

 * SIGTERM: it papers only over a general issue and should be dropped.

Fair enough.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] mingw: make mingw_signal return the correct handler

2013-06-09 Thread Johannes Sixt
From: Erik Faye-Lund kusmab...@gmail.com

Returning the SIGALRM handler for SIGINT is not very useful.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Signed-off-by: Johannes Sixt j...@kdbg.org
---
Am 6/7/2013 16:20, schrieb Erik Faye-Lund:
 On Fri, Jun 7, 2013 at 3:07 PM, Johannes Sixt j.s...@viscovery.net wrote:
 BTW, isn't mingw_signal() bogus in that it returns the SIGALRM handler
 even if a SIGINT handler is installed?
 
 Yep, that's a bug. Thanks for noticing.

This is your patch to address it.

 I've pushed out a branch here that tries to address these issues, but
 I haven't had time to test them. I'll post the series when I have. In
 the mean time:
 
 https://github.com/kusma/git/tree/win32-signal-raise

Concerning the other two patches:

* SIGINT: perhaps handle only the SIG_DFL case (for the exit code) and
forward all other cases to MSVCRT?

* SIGTERM: it papers only over a general issue and should be dropped.

IMO.

-- Hannes

 compat/mingw.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index b295e2f..bb92c43 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1677,14 +1677,16 @@ int sigaction(int sig, struct sigaction *in,
struct sigaction *out)
 #undef signal
 sig_handler_t mingw_signal(int sig, sig_handler_t handler)
 {
-   sig_handler_t old = timer_fn;
+   sig_handler_t old;

switch (sig) {
case SIGALRM:
+   old = timer_fn;
timer_fn = handler;
break;

case SIGINT:
+   old = sigint_fn;
sigint_fn = handler;
break;

-- 
1.8.3.1504.g78dbf7a
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html