[RFC] undefine SIGHUP if function kill not available...

[RFC] undefine SIGHUP if function kill not available...

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 2337 bytes --]

I have just seen Mark's comments (thank you!) about this patch, so I just
downgraded this request to a request for comments.

Here is the problem: 

In event-top.c, some code that is conditional on SIGHUP being defined
also uses kill(). On x86_64-windows, the MinGW signal.h now defines
SIGHUP, but kill is still not available.  So the approach I took for
now was to pretend that SIGHUP is not defined if kill is not available.
This is clearly a hack, hence the comment I added.

Mark's comments were:
> >  AC_CHECK_FUNCS([canonicalize_file_name realpath getrusage getuid \
> > -                getgid poll pread64 sbrk setpgid setpgrp setsid \
> > +                getgid kill poll pread64 sbrk setpgid setpgrp setsid \
> >               sigaction sigprocmask sigsetmask socketpair syscall \
> >               ttrace wborder setlocale])
> 
> Why the hell do we need to add a check for a function defined by ISO C90?
> 
> > +#if defined (SIGHUP) && !defined (HAVE_KILL)
> > +/* On x86_64-windows, MingW's signal.h defines SIGHUP but does not
> > +   provide "kill".  However, the code that uses SIGHUP below also
> > +   uses kill.  So, if kill is not available, pretend SIGHUP isn't
> > +   either.  */
> > +#undef SIGHUP
> > +#endif
> 
> This must be a bug in MingW.  Please tell the MingW people that in
> 2008 they manage to ship an environment that doesn't even implement
> ISO C90 correctly.  If they fix it (or have already fixed it) is it
> really necessary to add this ugly workaround?

Just to make sure I understand, my understanding is the fact that
"kill" is not available, right? Not knowing Windows all that well,
I wonder how much sense a "kill" routine would make.

That being said, I agree that, if MinGW has already fixed the problem,
it's reasonable to require a recent version of MinGW. I will double-check
for sure. But in the event that this is not the case, I think it's
reasonable to add the workaround. I should probably add a FIXME and a
date, to show that this should really be a temporary measure. Or maybe
there's a better way?

2009-01-07  Joel Brobecker  <brobecker@adacore.com>

        * configure.ac: Add kill to the list of functions to check.
        * configure, config.in: Regenerate.
        * event-top.c: Undefine SIGHUP if kill is not available.

Tested on x86-windows and x86-linux.

-- 
Joel

[-- Attachment #2: kill.diff --]
[-- Type: text/plain, Size: 1126 bytes --]

diff --git a/gdb/configure.ac b/gdb/configure.ac
index 4e0cf7d..e970234 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -774,7 +774,7 @@ AC_FUNC_ALLOCA
 AC_FUNC_MMAP
 AC_FUNC_VFORK
 AC_CHECK_FUNCS([canonicalize_file_name realpath getrusage getuid \
-                getgid poll pread64 sbrk setpgid setpgrp setsid \
+                getgid kill poll pread64 sbrk setpgid setpgrp setsid \
 		sigaction sigprocmask sigsetmask socketpair syscall \
 		ttrace wborder setlocale])
 
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 5483608..dd223e1 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -50,6 +50,14 @@ static void change_line_handler (void);
 static void change_annotation_level (void);
 static void command_handler (char *command);
 
+#if defined (SIGHUP) && !defined (HAVE_KILL)
+/* On x86_64-windows, MingW's signal.h defines SIGHUP but does not
+   provide "kill".  However, the code that uses SIGHUP below also
+   uses kill.  So, if kill is not available, pretend SIGHUP isn't
+   either.  */
+#undef SIGHUP
+#endif
+
 /* Signal handlers. */
 #ifdef SIGQUIT
 static void handle_sigquit (int sig);

Re: [RFC] undefine SIGHUP if function kill not available...

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1232 bytes --]

On Wed, Jan 07, 2009 at 03:58:32PM +0400, Joel Brobecker wrote:
> I have just seen Mark's comments (thank you!) about this patch, so I just
> downgraded this request to a request for comments.
> 
> Here is the problem: 
> 
> In event-top.c, some code that is conditional on SIGHUP being defined
> also uses kill(). On x86_64-windows, the MinGW signal.h now defines
> SIGHUP, but kill is still not available.  So the approach I took for
> now was to pretend that SIGHUP is not defined if kill is not available.
> This is clearly a hack, hence the comment I added.

Here is a new patch that replaces calls to "kill (getpid (), ...)"
by calls to "raise".

2009-01-08  Joel Brobecker  <brobecker@adacore.com>

        * event-top.c (async_disconnect, async_stop_sig): use "raise"
        instead of "kill" to raise a signal.

Tested on x86-linux, although I doubt that the testcase actually
exercises this part of the code. I tried to trigger the signal
handler that calls the "raise", but somehow it doesn't happen.
I wonder if this happens only when the inferior is still running...

In any case, it looks relatively straightforward, but I'd rather
have the confirmation from someone who's experienced with signals.

Thanks,
-- 
Joel

[-- Attachment #2: raise.diff --]
[-- Type: text/plain, Size: 609 bytes --]

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 5483608..fd7c521 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -977,7 +977,7 @@ async_disconnect (gdb_client_data arg)
 		"Could not kill the program being debugged",
 		RETURN_MASK_ALL);
   signal (SIGHUP, SIG_DFL);	/*FIXME: ??????????? */
-  kill (getpid (), SIGHUP);
+  raise (SIGHUP);
 }
 #endif
 
@@ -1005,7 +1005,7 @@ async_stop_sig (gdb_client_data arg)
 #elif HAVE_SIGSETMASK
   sigsetmask (0);
 #endif
-  kill (getpid (), SIGTSTP);
+  raise (SIGTSTP);
   signal (SIGTSTP, handle_stop_sig);
 #else
   signal (STOP_SIGNAL, handle_stop_sig);

Re: [RFC] undefine SIGHUP if function kill not available...

[Daniel Jacobowitz]

On Thu, Jan 08, 2009 at 05:51:31PM +0400, Joel Brobecker wrote:
> In any case, it looks relatively straightforward, but I'd rather
> have the confirmation from someone who's experienced with signals.

Looks fine to me.

-- 
Daniel Jacobowitz
CodeSourcery