[RFC 17/17] Simplify gdbserver's serial event handling

Tom Tromey
2019-02-24T16:52:08Z

Currently, when gdbserver handles a serial event, it also calls
'reschedule' to install a timer that is then used to process any data
that remains after the previous packet was processed.

It seemed better to me to simply have the file descriptor callback
loop, processing packets until complete.

2019-02-24 Tom Tromey <tom@tromey.com>

* server.h (handle_serial_event): Update.
* server.c (handle_serial_event): Return int. Remove parameters.
* remote-utils.c (readchar_callback): Remove.
(handle_accept_event, remote_open): Use handle_all_serial_events.
(readchar): Don't call reschedule.
(reset_readchar): Update.
(process_remaining, reschedule): Remove.
(handle_all_serial_events): New function.
---
gdb/gdbserver/ChangeLog | 11 ++++++++
gdb/gdbserver/remote-utils.c | 53 +++++++++++++-----------------------
gdb/gdbserver/server.c | 10 +++----
gdb/gdbserver/server.h | 2 +-
4 files changed, 35 insertions(+), 41 deletions(-)

diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index b1e4e869b69..7b41090efcc 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -83,13 +83,9 @@ enum {
NOT_SCHEDULED = -1
};

-/* Status of the readchar callback.
- Either NOT_SCHEDULED or the callback id. */
-static int readchar_callback = NOT_SCHEDULED;
-
static int readchar (void);
static void reset_readchar (void);
-static void reschedule (void);
+static void handle_all_serial_events (int err, gdb_client_data client_data);

/* A cache entry for a successfully looked-up symbol. */
struct sym_cache
@@ -204,7 +200,7 @@ handle_accept_event (int err, gdb_client_data client_data)
enable_async_notification (remote_desc);

/* Register the event loop handler. */
- add_file_handler (remote_desc, handle_serial_event, NULL);
+ add_file_handler (remote_desc, handle_all_serial_events, NULL);

/* We have a new GDB connection now. If we were disconnected
tracing, there's a window where the target could report a stop
@@ -341,7 +337,7 @@ remote_open (const char *name)
enable_async_notification (remote_desc);

/* Register the event loop handler. */
- add_file_handler (remote_desc, handle_serial_event, NULL);
+ add_file_handler (remote_desc, handle_all_serial_events, NULL);
}
#ifndef USE_WIN32API
else if (port_str == NULL)
@@ -382,7 +378,7 @@ remote_open (const char *name)
enable_async_notification (remote_desc);

/* Register the event loop handler. */
- add_file_handler (remote_desc, handle_serial_event, NULL);
+ add_file_handler (remote_desc, handle_all_serial_events, NULL);
}
#endif /* USE_WIN32API */
else
@@ -879,9 +875,9 @@ initialize_async_io (void)
#endif
}

-/* Internal buffer used by readchar.
- These are global to readchar because reschedule_remote needs to be
- able to tell whether the buffer is empty. */
+/* Internal buffer used by readchar. These are global to readchar
+ because handle_all_serial_events needs to be able to tell whether
+ the buffer is empty. */

static unsigned char readchar_buf[BUFSIZ];
static int readchar_bufcnt = 0;
@@ -916,7 +912,6 @@ readchar (void)

readchar_bufcnt--;
ch = *readchar_bufp++;
- reschedule ();
return ch;
}

@@ -926,33 +921,23 @@ static void
reset_readchar (void)
{
readchar_bufcnt = 0;
- if (readchar_callback != NOT_SCHEDULED)
- {
- delete_timer (readchar_callback);
- readchar_callback = NOT_SCHEDULED;
- }
}

-/* Process remaining data in readchar_buf. */
+/* Loop, calling handle_serial_event, until there is no more data
+ available. */

static void
-process_remaining (void *context)
+handle_all_serial_events (int err, gdb_client_data client_data)
{
- /* This is a one-shot event. */
- readchar_callback = NOT_SCHEDULED;
-
- if (readchar_bufcnt > 0)
- handle_serial_event (0, NULL);
-}
-
-/* If there is still data in the buffer, queue another event to process it,
- we can't sleep in select yet. */
-
-static void
-reschedule (void)
-{
- if (readchar_bufcnt > 0 && readchar_callback == NOT_SCHEDULED)
- readchar_callback = create_timer (0, process_remaining, NULL);
+ do
+ {
+ if (handle_serial_event () < 0)
+ {
+ stop_event_loop ();
+ break;
+ }
+ }
+ while (readchar_bufcnt > 0);
}

/* Read a packet from the remote machine, with error checking,
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 42147c1d836..3732d53cfbc 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -4389,22 +4389,20 @@ process_serial_event (void)

/* Event-loop callback for serial events. */

-void
-handle_serial_event (int err, gdb_client_data client_data)
+int
+handle_serial_event ()
{
if (debug_threads)
debug_printf ("handling possible serial event\n");

/* Really handle it. */
if (process_serial_event () < 0)
- {
- stop_event_loop ();
- return;
- }
+ return -1;

/* Be sure to not change the selected thread behind GDB's back.
Important in the non-stop mode asynchronous protocol. */
set_desired_thread ();
+ return 0;
}

/* Push a stop notification on the notification queue. */
diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index e6a8dcc9bf1..9ffc9e0c958 100644
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -83,7 +83,7 @@ extern int non_stop;
/* Functions from server.c. */
extern void handle_v_requests (char *own_buf, int packet_len,
int *new_packet_len);
-extern void handle_serial_event (int err, gdb_client_data client_data);
+extern int handle_serial_event ();
extern void handle_target_event (int err, gdb_client_data client_data);

/* Get rid of the currently pending stop replies that match PTID. */
--
2.17.2