From 4e5dbecd9b1ac780181e04f8b51a4cd133c4cdbe Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Mon, 31 Jan 2022 10:51:49 +0100 Subject: [PATCH] keep-alive: drop link with log framework OpenOCD implements the GDB keep-alive by sending empty strings as output for GDB client. This has been implemented as part of the log framework, creating an odd dependency. Move the keep-alive notifications out of log framework. For the moment, keep keep_alive() inside log.c, but it should be moved in server.c This should also fix an old issue with KDE Konsole when tab alert for activity is enabled. The empty strings is sent to all the connections, including telnet, and causes the tab running OpenOCD telnet to continuously show activity even when no new text is printed. Anyway, I cannot replicate this issue anymore. Change-Id: Iebb00b00fb74b3c9665d9e1ddd3c055275bfbd43 Signed-off-by: Antonio Borneo Reviewed-on: https://review.openocd.org/c/openocd/+/6840 Reviewed-by: Tomas Vanek Tested-by: jenkins --- src/helper/log.c | 36 ++++++++++++++++-------------------- src/server/gdb_server.c | 24 +++++++++++++++++++++++- src/server/server.c | 8 ++++++++ src/server/server.h | 2 ++ 4 files changed, 49 insertions(+), 21 deletions(-) diff --git a/src/helper/log.c b/src/helper/log.c index 12f1790e9..106d22867 100644 --- a/src/helper/log.c +++ b/src/helper/log.c @@ -30,6 +30,7 @@ #include "command.h" #include "replacements.h" #include "time_support.h" +#include #include @@ -110,32 +111,27 @@ static void log_puts(enum log_levels level, if (f) file = f + 1; - if (strlen(string) > 0) { - if (debug_level >= LOG_LVL_DEBUG) { - /* print with count and time information */ - int64_t t = timeval_ms() - start; + if (debug_level >= LOG_LVL_DEBUG) { + /* print with count and time information */ + int64_t t = timeval_ms() - start; #ifdef _DEBUG_FREE_SPACE_ - struct mallinfo info; - info = mallinfo(); + struct mallinfo info; + info = mallinfo(); #endif - fprintf(log_output, "%s%d %" PRId64 " %s:%d %s()" + fprintf(log_output, "%s%d %" PRId64 " %s:%d %s()" #ifdef _DEBUG_FREE_SPACE_ - " %d" + " %d" #endif - ": %s", log_strings[level + 1], count, t, file, line, function, + ": %s", log_strings[level + 1], count, t, file, line, function, #ifdef _DEBUG_FREE_SPACE_ - info.fordblks, + info.fordblks, #endif - string); - } else { - /* if we are using gdb through pipes then we do not want any output - * to the pipe otherwise we get repeated strings */ - fprintf(log_output, "%s%s", - (level > LOG_LVL_USER) ? log_strings[level + 1] : "", string); - } + string); } else { - /* Empty strings are sent to log callbacks to keep e.g. gdbserver alive, here we do - *nothing. */ + /* if we are using gdb through pipes then we do not want any output + * to the pipe otherwise we get repeated strings */ + fprintf(log_output, "%s%s", + (level > LOG_LVL_USER) ? log_strings[level + 1] : "", string); } fflush(log_output); @@ -452,7 +448,7 @@ void keep_alive(void) last_time = current_time; /* this will keep the GDB connection alive */ - LOG_USER_N("%s", ""); + server_keep_clients_alive(); /* DANGER!!!! do not add code to invoke e.g. target event processing, * jim timer processing, etc. it can cause infinite recursion + diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c index b47e312a3..f5736196e 100644 --- a/src/server/gdb_server.c +++ b/src/server/gdb_server.c @@ -3673,13 +3673,35 @@ static int gdb_input(struct connection *connection) return ERROR_OK; } +static void gdb_keep_client_alive(struct connection *connection) +{ + struct gdb_connection *gdb_con = connection->priv; + + if (gdb_con->busy) { + /* do not send packets, retry asap */ + return; + } + + switch (gdb_con->output_flag) { + case GDB_OUTPUT_NO: + /* no need for keep-alive */ + break; + case GDB_OUTPUT_ALL: + /* send an empty O packet */ + gdb_output_con(connection, ""); + break; + default: + break; + } +} + static const struct service_driver gdb_service_driver = { .name = "gdb", .new_connection_during_keep_alive_handler = NULL, .new_connection_handler = gdb_new_connection, .input_handler = gdb_input, .connection_closed_handler = gdb_connection_closed, - .keep_client_alive_handler = NULL, + .keep_client_alive_handler = gdb_keep_client_alive, }; static int gdb_target_start(struct target *target, const char *port) diff --git a/src/server/server.c b/src/server/server.c index 4ec196728..dd408048c 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -421,6 +421,14 @@ static int remove_services(void) return ERROR_OK; } +void server_keep_clients_alive(void) +{ + for (struct service *s = services; s; s = s->next) + if (s->keep_client_alive) + for (struct connection *c = s->connections; c; c = c->next) + s->keep_client_alive(c); +} + int server_loop(struct command_context *command_context) { struct service *service; diff --git a/src/server/server.h b/src/server/server.h index 00f1a428f..a6b1963a6 100644 --- a/src/server/server.h +++ b/src/server/server.h @@ -106,6 +106,8 @@ int server_quit(void); void server_free(void); void exit_on_signal(int sig); +void server_keep_clients_alive(void); + int server_loop(struct command_context *command_context); int server_register_commands(struct command_context *context);