This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [Patch] Win32 gdbserver new interrupt support, and attach to process fix.
Pedro Alves wrote:
Lerele wrote:
}
-#endif
+
+/* Expose 'static void input_interrupt (int unused)' function to
enable checking for a + remote interrupt request. */
Is the 'Expose 'static void input_interrupt (int unused)' function'
part really necessary?
You mean the comment?
The function?
Or even having to call that function at all to be able to interrupt?
I added a comment just to state that it's a wrapper for that other (with
a not so significant function name, given its purpose for win32).
I added the function:
1) Not to change static attribute input_interrupt has (I am of the kind
that I prefer not touch stuff if it's not necessary --less possibility
for new bugs I think).
2) To give it a rather more significant name (given the context in which
it is called now), and not expose it as 'input_interrupt'.
Having to call that function is necessary to check if client gdb asked
gdbserver to stop child process.
Also, can't use async IO signals on win32 the way it's done on
gdbserver, so I just directly "exposed" that function.
+void
+check_remote_input_interrupt_request (void)
+{
+ input_interrupt(0);
+}
+
+ It should be safe to continue child given this wait status.
+ See function get_child_debug_event. Default wait status is
spurious,
+ and it gets modified if any important debug events get received.
+ More specifically, this status gets returned in the wait loop
to + allow socket pooling/resuming, to allow for remote
interruption. */
Full stop, double space. "...get_child_debug_event. Default ..."
Please clarify.
TARGET_WAITKIND_SPURIOUS,
};
@@ -574,7 +584,7 @@
FreeLibrary (kernel32);
- return res;
+ return res? 0:-1;
Space around '?' and ':'. Personally, I liked the res != FALSE ? 0 :
-1 version better.
Honestly, I don't really care too much.
You guys decide.
}
/* Kill all inferiors. */
@@ -840,7 +850,12 @@
last_sig = TARGET_SIGNAL_0;
ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
- if (!(debug_event = WaitForDebugEvent (¤t_event, 1000)))
+ /* Check for remote interruption request */
+ check_remote_input_interrupt_request();
Missing space: "check_remote_input_interrupt_request ()"
Full stop double-space: "request. */", although I guess the comment
adds no value, since the
function name says pretty much the same.
+/* Send a signal to the inferior process, however is appropriate. */
+static void
+win32_send_signal (int signo)
+{
+ if ( signo == TARGET_SIGNAL_INT )
+ {
+ if ( !GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT,
current_process_id) )
Missing space: "GenerateConsoleCtrlEvent ("
Also, pedantically speaking, both remote_utils.c:putpkt_binary, and
remote_utils.c:input_interrupt
(the sole users of _send_signal) send SIGINT, not TARGET_SIGNAL_INT.
Luckily on
Windows, both are defined as 2.
I have a local patch that does:
/* Send a signal to the inferior process, however is appropriate. */
- void (*send_signal) (int);
+ void (*send_signal) (enum target_signal);
... and takes care of the conversion of the target side. I'll post it
for review. In the meantime, you may
want to change your patch to use SIGINT.
You mean line gdb/signals/signal.c 'target_signal_from_host' and
'do_target_signal_to_host' functions?
I'm not really sure if we should use SIGINT in win32-i386-low.c, given
that it should only know about target signals.
Thanks for such a needed feature.
Thanks to you.
Nice to see it gets used.
I surely find it useful. It's just great to cross develop on Linux to
just use Windows as a target. 8-)
However, I don't know if people out there are using it, otherwise I
guess the attach bug should have been listed.
It isn't that strange to use the attach feature, is it? Well, I don't
use it really. hmmm.
Been taking a look ocasionally at gdb buglist, but didn't see any related.
Regards,
Leo.