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

4 files changed, 49 insertions, 21 deletions

diff --git a/src/helper/log.c b/src/helper/log.c
index 12f1790..106d228 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 <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 b47e312..f573619 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 4ec1967..dd40804 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 00f1a42..a6b1963 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);