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 <borneo.antonio@gmail.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/6840
Reviewed-by: Tomas Vanek <vanekt@fbl.cz>
Tested-by: jenkins

diff --git a/src/helper/log.c b/src/helper/log.c
index 12f1790e977d6b5c722f345fad49f6fc4a316150..106d228675d012a08a4bc71ebeced21fbfc63ab9 100644 (file)
--- 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 <server/server.h>
 
 #include <stdarg.h>
 
@@ -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 b47e312a3d35e402854dae04f7af83663dd61579..f5736196eddfb4717d389688d631c1e76f31bceb 100644 (file)
--- 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 4ec196728ae4566470726d73f2ec2746915651c7..dd408048cf1736dabc94f4fe6ca8c42af6ee4d1e 100644 (file)
--- 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 00f1a428ff2b37980f68d82c000e5c93d88f033d..a6b1963a67b009f9c85ec6ead5d7563ff19203fa 100644 (file)
--- 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);