* [patch] MI telnet service
@ 2012-07-12 11:21 Abid, Hafiz
2012-07-12 13:19 ` Gary Benson
` (3 more replies)
0 siblings, 4 replies; 20+ messages in thread
From: Abid, Hafiz @ 2012-07-12 11:21 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1805 bytes --]
Hi,
This patch provides implementation of telnet service. This is based on initial work by Grigory Tolstolytkin.(http://sourceware.org/ml/gdb-patches/2011-11/msg00466.html)
This service can accept and execute CLI commands from the remote user. It can be started by MI command "-start-telnet-service [port]". After the service is running, user can connect to the GDB via telnet and execute CLI commands in parallel with existing MI interface. This service can be stopped by another MI command "-stop-telnet-service". For these commands to work, GDB has to be configured with --enable-gdbmitel=yes.
This service can be useful, for example, when GDB is running as a backend of an IDE. Testing infrastructure can run the tests by sending commands through socket. In case of an error, a user can debug the problem in the IDE.
The test case that I added only checks that port is opened after the service is started. I am looking to improve it and would appreciate if the reviewers can help me with some ideas.
How does it all look?
Regards,
Abid
2012-07-12 Hafiz Abid Qadeer <abidh@codesourcery.com>
* Makefile.in: Variable definitions related to mi-telnet.c.
(mi-telnet.o): New rule.
* configure.ac: Add AC_ARG_ENABLE(gdbmitel).
* configure: Generated.
* mi/mi-cmds.c (mi_cmds): Add -start-telnet-service
and stop-telnet-service MI commands.
* mi/mi-cmds.h (mi_cmd_start_telnet_service): Add declaration.
(mi_cmd_stop_telnet_service): Add declaration.
* mi/mi-telnet.c: New file.
* gdb/testsuite:
gdb.mi/mi-telnet.exp: New file
gdb.mi/mi-telnet.c: New file
* gdb.texinfo (Miscellaneous GDB/MI Commands): Added
-start-telnet-service and -stop-telnet-service.
[-- Attachment #2: mi-telnet.patch --]
[-- Type: application/octet-stream, Size: 29537 bytes --]
Index: gdb/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.1210
diff -u -r1.1210 Makefile.in
--- gdb/Makefile.in 2 Jul 2012 15:29:33 -0000 1.1210
+++ gdb/Makefile.in 12 Jul 2012 10:35:11 -0000
@@ -216,6 +216,15 @@
SUBDIR_MI_CFLAGS=
#
+# MI telnet definitions
+#
+SUBDIR_MITEL_OBS = mi-telnet.o
+SUBDIR_MITEL_SRCS = mi/mi-telnet.c
+SUBDIR_MITEL_DEPS=
+SUBDIR_MITEL_LDFLAGS=
+SUBDIR_MITEL_CFLAGS= -DENABLE_GDBMITEL
+
+#
# TUI sub directory definitions
#
@@ -1894,6 +1903,10 @@
$(COMPILE) $(srcdir)/mi/mi-common.c
$(POSTCOMPILE)
+mi-telnet.o: $(srcdir)/mi/mi-telnet.c
+ $(COMPILE) $(srcdir)/mi/mi-telnet.c
+ $(POSTCOMPILE)
+
# gdb/common/ dependencies
#
# Need to explicitly specify the compile rule as make will do nothing
Index: gdb/configure
===================================================================
RCS file: /cvs/src/src/gdb/configure,v
retrieving revision 1.370
diff -u -r1.370 configure
--- gdb/configure 2 Jul 2012 20:31:07 -0000 1.370
+++ gdb/configure 12 Jul 2012 10:35:13 -0000
@@ -784,6 +784,7 @@
enable_64_bit_bfd
enable_gdbcli
enable_gdbmi
+enable_gdbmitel
enable_tui
enable_gdbtk
with_libunwind_ia64
@@ -1462,6 +1463,7 @@
--enable-64-bit-bfd 64-bit support (on hosts with narrower word sizes)
--disable-gdbcli disable command-line interface (CLI)
--disable-gdbmi disable machine-interface (MI)
+ --enable-gdbmitel enable remote interface over telnet
--enable-tui enable full-screen terminal user interface (TUI)
--enable-gdbtk enable gdbtk graphical user interface (GUI)
--enable-profiling enable profiling of GDB
@@ -5237,6 +5239,28 @@
fi
fi
+# Enable MI telnet.
+# Check whether --enable-gdbmitel was given.
+if test "${enable_gdbmitel+set}" = set; then :
+ enableval=$enable_gdbmitel; case $enableval in
+ yes | no)
+ ;;
+ *)
+ as_fn_error "bad value $enableval for --enable-gdbmitel" "$LINENO" 5 ;;
+ esac
+else
+ enable_gdbmitel=no
+fi
+
+if test x"$enable_gdbmitel" = xyes; then
+ if test -d $srcdir/mi; then
+ CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_MITEL_OBS)"
+ CONFIG_DEPS="$CONFIG_DEPS \$(SUBDIR_MITEL_DEPS)"
+ CONFIG_SRCS="$CONFIG_SRCS \$(SUBDIR_MITEL_SRCS)"
+ ENABLE_CFLAGS="$ENABLE_CFLAGS \$(SUBDIR_MITEL_CFLAGS)"
+ fi
+fi
+
# Enable TUI.
# Check whether --enable-tui was given.
if test "${enable_tui+set}" = set; then :
Index: gdb/configure.ac
===================================================================
RCS file: /cvs/src/src/gdb/configure.ac,v
retrieving revision 1.181
diff -u -r1.181 configure.ac
--- gdb/configure.ac 2 Jul 2012 20:31:09 -0000 1.181
+++ gdb/configure.ac 12 Jul 2012 10:35:13 -0000
@@ -329,6 +329,25 @@
fi
fi
+# Enable MI telnet.
+AC_ARG_ENABLE(gdbmitel,
+AS_HELP_STRING([--enable-gdbmitel], [enable remote interface over telnet]),
+ [case $enableval in
+ yes | no)
+ ;;
+ *)
+ AC_MSG_ERROR([bad value $enableval for --enable-gdbmitel]) ;;
+ esac],
+ [enable_gdbmitel=no])
+if test x"$enable_gdbmitel" = xyes; then
+ if test -d $srcdir/mi; then
+ CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_MITEL_OBS)"
+ CONFIG_DEPS="$CONFIG_DEPS \$(SUBDIR_MITEL_DEPS)"
+ CONFIG_SRCS="$CONFIG_SRCS \$(SUBDIR_MITEL_SRCS)"
+ ENABLE_CFLAGS="$ENABLE_CFLAGS \$(SUBDIR_MITEL_CFLAGS)"
+ fi
+fi
+
# Enable TUI.
AC_ARG_ENABLE(tui,
AS_HELP_STRING([--enable-tui], [enable full-screen terminal user interface (TUI)]),
Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.990
diff -u -r1.990 gdb.texinfo
--- gdb/doc/gdb.texinfo 5 Jul 2012 01:08:24 -0000 1.990
+++ gdb/doc/gdb.texinfo 12 Jul 2012 10:35:16 -0000
@@ -32892,6 +32892,61 @@
(gdb)
@end smallexample
+@subheading The @code{-start-telnet-service} Command
+@findex -start-telnet-service
+
+@subheading Synopsis
+
+@smallexample
+-start-telnet-service [port]
+@end smallexample
+
+Start a telnet service on the given port. After the service is started,
+user can connect to the gdb via telnet and execute CLI commands in
+parallel with existing MI interface. If no @samp{port} argument is given
+then default port 9950 is used. Only one service is allowed at one time.
+So user will have to stop the already running service before starting a
+new one. For telnet service to work, @value{GDBN}
+should be configured with --enable-gdbmitel=yes.
+
+@subheading @value{GDBN} Command
+
+No equivalent.
+
+@subheading Example
+
+@smallexample
+(gdb)
+-start-telnet-service 8002
+^done
+(gdb)
+@end smallexample
+
+@subheading The @code{-stop-telnet-service} Command
+@findex -stop-telnet-service
+
+@subheading Synopsis
+
+@smallexample
+-stop-telnet-service
+@end smallexample
+
+Stop the telnet service if it is already started. Otherwise this
+command has no effect.
+
+@subheading @value{GDBN} Command
+
+No equivalent.
+
+@subheading Example
+
+@smallexample
+(gdb)
+-stop-telnet-service
+^done
+(gdb)
+@end smallexample
+
@node Annotations
@chapter @value{GDBN} Annotations
Index: gdb/mi/mi-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmds.c,v
retrieving revision 1.60
diff -u -r1.60 mi-cmds.c
--- gdb/mi/mi-cmds.c 24 May 2012 00:33:46 -0000 1.60
+++ gdb/mi/mi-cmds.c 12 Jul 2012 10:35:16 -0000
@@ -137,6 +137,10 @@
{ "var-show-attributes", { NULL, 0 }, mi_cmd_var_show_attributes},
{ "var-show-format", { NULL, 0 }, mi_cmd_var_show_format},
{ "var-update", { NULL, 0 }, mi_cmd_var_update},
+#ifdef ENABLE_GDBMITEL
+ { "start-telnet-service", { NULL, 0 }, mi_cmd_start_telnet_service},
+ { "stop-telnet-service", { NULL, 0 }, mi_cmd_stop_telnet_service},
+#endif
{ NULL, }
};
Index: gdb/mi/mi-cmds.h
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmds.h,v
retrieving revision 1.55
diff -u -r1.55 mi-cmds.h
--- gdb/mi/mi-cmds.h 24 May 2012 00:33:46 -0000 1.55
+++ gdb/mi/mi-cmds.h 12 Jul 2012 10:35:16 -0000
@@ -118,6 +118,10 @@
extern mi_cmd_argv_ftype mi_cmd_var_update;
extern mi_cmd_argv_ftype mi_cmd_enable_pretty_printing;
extern mi_cmd_argv_ftype mi_cmd_var_set_update_range;
+#ifdef ENABLE_GDBMITEL
+extern mi_cmd_argv_ftype mi_cmd_start_telnet_service;
+extern mi_cmd_argv_ftype mi_cmd_stop_telnet_service;
+#endif
/* Description of a single command. */
Index: gdb/mi/mi-telnet.c
===================================================================
RCS file: gdb/mi/mi-telnet.c
diff -N gdb/mi/mi-telnet.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ gdb/mi/mi-telnet.c 12 Jul 2012 10:35:16 -0000
@@ -0,0 +1,670 @@
+/* MI telnet support
+
+ Copyright (C) 2000, 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2009, 2010,
+ 2011, 2012 Free Software Foundation, Inc.
+
+ Contributed by Mentor Graphics Corporation <www.mentor.com>
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+/* MI telnet support allows remote user to connect to gdb using telnet.
+ GDB should be built with "--enable-gdbmitel=yes" option to enable MI telnet.
+
+ To Start the service use MI command "-start-telnet-service [port]".
+ Default port value is 9950.
+ Service can be stopped by issuing "-stop-telnet-service" command.
+
+ The following options are required for interactive commands
+ and can be turned on directly via telnet on the fly:
+ "set confirm on"
+ "set interactive on"
+*/
+
+#include "defs.h"
+#include <sys/socket.h>
+#include <netinet/in.h>
+
+#include "top.h"
+#include "mi-cmds.h"
+#include "gdb_string.h"
+#include "event-loop.h"
+#include "interps.h"
+#include "main.h"
+#include "breakpoint.h"
+#include "cli/cli-cmds.h"
+#include "cli/cli-decode.h"
+#include "readline/readline.h"
+#include "readline/history.h"
+
+#define DEFAULT_PORT 9950
+#define MAX_CMD_LENGTH 512
+
+/* Hook to ask remote client for
+ additional 'yes or no' question. */
+static int telnet_query_hook (const char *msg, va_list argp);
+
+/* Execute telnet command. */
+static void execute_telnet_command (char* line);
+
+/* This is called when data is available on telnet port. */
+static void get_input_from_telnet (int err, gdb_client_data client_data);
+
+/* Accept client connection and register it
+ with event_loop to get data from telnet client. */
+static void telnet_accept_handler(int err, gdb_client_data);
+
+/* Remove the socket descriptor from the event handler and close it. */
+static void close_socket_desc (int *fd);
+
+/* A utility function to receive data from socket
+ and do error checking. */
+static int receive (int *fd, void *buf,
+ size_t n, int flags);
+
+/* A utility function to get a line of input from socket. */
+static int receive_line (int *fd, char* buf, int buf_size);
+
+/* A utility function to send the output to client. */
+static void reply_msg (const char *msg);
+
+/* Sends the reply to remote client with extra '\r' character
+ for every '\n' in the msg so that it will show up correctly on
+ telnet console. */
+static void reply_msg_with_carriage_return (const char *msg);
+
+/* Setup the hooks to do most of the communication work. */
+static void setup_hooks (int);
+
+/* Socket for accepting telnet connections. */
+static int telnet_s = -1;
+
+/* Client connection descriptor. */
+static int client_fd = -1;
+
+/* For output redirecting. */
+static struct ui_file *str_file = NULL;
+
+/* The Prompt to show on telnet. */
+static char* mi_prompt = "";
+
+/* Hooks which are called at various points during telnet
+ command processing. */
+
+/* This is called when we are about to process a command. */
+int (*cmd_start_hook) (int *fd);
+
+/* This is called when we have processed a command. */
+int (*cmd_end_hook) (int *fd);
+
+/* This is called after connection is complete to setup
+ the mode. */
+int (*init_mode_hook) (int *fd);
+
+/* This is called when data is avaialble on the socket. */
+void (*data_available_hook) (int *fd);
+
+/* This is called when we are closeing telnet service. */
+void (*stop_service_hook) ();
+
+/* This is called when user need to respond to a question. */
+char (*get_answer_hook) (int *fd);
+
+/* This is called when user needs to provide some input, mostly for
+ getting list of commands. */
+char* (*get_input_hook) (int *fd, char* prompt);
+
+
+/* Command to start listening for client on specified port. */
+
+void
+mi_cmd_start_telnet_service (char *command, char **argv, int argc)
+{
+ struct sockaddr_in serv, client;
+ socklen_t len;
+ int port;
+ int val;
+ char* tmp_name;
+
+ if (telnet_s != -1 || client_fd != -1)
+ {
+ error (_("Error: telnet service already started"));
+ return;
+ }
+
+ if (argc != 0 && argc != 1)
+ {
+ error (_("Usage: -start_telnet_service [port]"));
+ return;
+ }
+
+ if (argc == 1)
+ port = atoi (argv[0]);
+ else
+ port = DEFAULT_PORT;
+
+ if (port <= 0)
+ {
+ error (_("Error: wrong port value: %d"), port);
+ return;
+ }
+
+ /* Setup server side of telnet service. */
+ memset ((char *)&serv, 0, sizeof (serv));
+ serv.sin_family = AF_INET;
+ serv.sin_port = htons (port);
+ serv.sin_addr.s_addr = htonl (INADDR_ANY);
+
+ telnet_s = socket (AF_INET, SOCK_STREAM, IPPROTO_TCP);
+ if (telnet_s == -1)
+ {
+ error (_("Error: socket can't be created"));
+ return;
+ }
+
+ if (bind (telnet_s, (const struct sockaddr *)&serv, sizeof (serv)) == -1)
+ {
+ close (telnet_s);
+ telnet_s = -1;
+ error (_("Error: port %d can't be bind"), port);
+ return;
+ }
+
+ /* one client so far allowed */
+ if (listen (telnet_s, 1) == -1)
+ {
+ close (telnet_s);
+ telnet_s = -1;
+ error (_("Error: %s"), safe_strerror (errno));
+ return;
+ }
+
+ /* Allocate file for output. */
+ if (!str_file)
+ str_file = mem_fileopen ();
+
+ mi_prompt = get_prompt ();
+
+ /* Setup the hooks to handle various situations. */
+ setup_hooks (0);
+
+ /* Register handler that will accept connections. */
+ add_file_handler (telnet_s, telnet_accept_handler, 0);
+}
+
+/* Command to stop listening for remote clients. */
+
+void
+mi_cmd_stop_telnet_service (char *command, char **argv, int argc)
+{
+ close_socket_desc (&client_fd);
+
+ close_socket_desc (&telnet_s);
+
+ if(str_file != NULL)
+ {
+ ui_file_delete (str_file);
+ str_file = NULL;
+ }
+
+ if (stop_service_hook)
+ (*stop_service_hook) ();
+}
+
+/* Hook for query to ask remote client for
+ additional 'yes or no' question. */
+
+static int
+telnet_query_hook (const char *msg, va_list argp)
+{
+ int retval = 0;
+ char *question = NULL;
+ char cmd;
+
+ gdb_flush (gdb_stdout);
+ question = xstrvprintf (msg, argp);
+
+ reply_msg_with_carriage_return (question);
+ reply_msg ("(y or n) ");
+
+ if(get_answer_hook)
+ {
+ cmd = (*get_answer_hook) (&client_fd);
+
+ /* Check the first letter in the answer. */
+ switch (cmd)
+ {
+ case 'y':
+ case 'Y':
+ retval = 1;
+ break;
+
+ case 'n':
+ case 'N':
+ default:
+ retval = 0;
+ break;
+ }
+ }
+
+ xfree (question);
+ return retval;
+}
+
+/* This function is called when GDB needs to get the list of
+ commands to run on breakpoints. */
+
+static char *
+telnet_readline_hook (char *prompt)
+{
+ if (get_input_hook)
+ return (*get_input_hook) (&client_fd, prompt);
+ else
+ return "";
+};
+
+/* Execute telnet command. */
+
+static void
+execute_telnet_command (char* line)
+{
+ struct gdb_exception e;
+ char *result = NULL;
+ struct cleanup *cleanup;
+ struct interp *old_interp = NULL;
+ struct interp *interp_to_use = NULL;
+ HIST_ENTRY *hist = NULL;
+ long len = 0;
+ void* old_query_hook;
+ void* old_readline_hook;
+
+ if (line == NULL)
+ return;
+
+ /* If this is a valid command then add it to history. */
+ if (*line != '\0' )
+ add_history (line);
+ else
+ {
+ /* This is an empty command. Look in the history. If nothing is
+ available in history then function will return. */
+ hist = previous_history ();
+ if ( (hist != NULL) && (hist->line != NULL) && (*hist->line != '\0') )
+ line = hist->line;
+ else
+ return;
+ }
+
+ if (cmd_start_hook)
+ (*cmd_start_hook) (&client_fd);
+
+ /* setup cleanups for input/output. */
+ cleanup = make_cleanup (null_cleanup, NULL);
+ make_cleanup_restore_ui_file (&gdb_stdin);
+ make_cleanup_restore_ui_file (&gdb_stdout);
+ make_cleanup_restore_ui_file (&gdb_stderr);
+ make_cleanup_restore_ui_file (&gdb_stdlog);
+ make_cleanup_restore_ui_file (&gdb_stdtarg);
+ make_cleanup_restore_ui_file (&gdb_stdtargerr);
+
+ /* execute command in sync, set batch mode to off */
+ make_cleanup_restore_integer (&batch_flag);
+ batch_flag = 0;
+
+ /* figure out current interpreter */
+ if (current_interp_named_p (INTERP_MI))
+ {
+ old_interp = interp_lookup (INTERP_MI);
+ }
+ else if (current_interp_named_p (INTERP_MI1))
+ {
+ old_interp = interp_lookup (INTERP_MI1);
+ }
+ else if (current_interp_named_p (INTERP_MI2))
+ {
+ old_interp = interp_lookup (INTERP_MI2);
+ }
+ else if (current_interp_named_p (INTERP_MI3))
+ {
+ old_interp = interp_lookup (INTERP_MI3);
+ }
+
+ /* switch to console interp */
+ interp_to_use = interp_lookup (INTERP_CONSOLE);
+ if (!interp_set (interp_to_use, 0))
+ {
+ reply_msg ("Error: unexpectedly failed to use CLI the interpreter\r\n");
+ do_cleanups (cleanup);
+ return;
+ }
+
+ /* redirect and catch all input/output into string */
+ if (ui_out_redirect (current_uiout, str_file) < 0)
+ reply_msg ("Error: unexpectedly failed to redirect user interface. "
+ "No command results will be print\r\n");
+ else
+ make_cleanup_ui_out_redirect_pop (current_uiout);
+
+ gdb_stdout = str_file;
+ gdb_stderr = str_file;
+ gdb_stdlog = str_file;
+ gdb_stdtarg = str_file;
+ gdb_stdtargerr = str_file;
+
+ /* Hook for command that may ask user for questions. */
+ old_query_hook = deprecated_query_hook;
+ old_readline_hook = deprecated_readline_hook;
+ deprecated_query_hook = telnet_query_hook;
+ deprecated_readline_hook = telnet_readline_hook;
+
+ /* Console interp is properly set up, execute the command. */
+ TRY_CATCH (e, RETURN_MASK_ALL)
+ {
+ execute_command (line, 1);
+ }
+
+ /* Do any breakpoint-related stuff. */
+ bpstat_do_actions ();
+
+ /* Restore the hooks. */
+ deprecated_query_hook = old_query_hook;
+ deprecated_readline_hook = old_readline_hook;
+
+ gdb_flush (str_file);
+
+ if (e.reason < 0)
+ {
+ reply_msg ("Error: ");
+ reply_msg (e.message);
+ reply_msg ("\r\n");
+ }
+
+ /* Send the reply to the client. */
+ result = ui_file_xstrdup (str_file, &len);
+
+ if (len > 0)
+ {
+ reply_msg_with_carriage_return (result);
+ }
+
+ xfree (result);
+
+ /* Clear output buffer and restore GDB settings. */
+ ui_file_rewind (str_file);
+ do_cleanups (cleanup);
+
+ /* Get back to MI interp. */
+ interp_set (old_interp, 0);
+
+ if (cmd_end_hook)
+ (*cmd_end_hook) (&client_fd);
+}
+
+/* This function is called when data is ready on teh socket and calls
+ the corresponding hook. */
+static void
+get_input_from_telnet (int err, gdb_client_data client_data)
+{
+ if (data_available_hook)
+ (*data_available_hook) (&client_fd);
+}
+
+/* Handler for the listening socket.
+ Accepts client connection and registers it with event_loop. */
+
+static void
+telnet_accept_handler(int err, gdb_client_data client_data)
+{
+ struct sockaddr_in client;
+ socklen_t len;
+ int s = -1;
+
+ if (client_fd != -1)
+ return; /* Another client is already connected. */
+
+ s = accept (telnet_s, (struct sockaddr *)&client, &len);
+ if (s != -1)
+ {
+ client_fd = s;
+
+ if (init_mode_hook)
+ {
+ if ((*init_mode_hook) (&client_fd) == 0)
+ {
+ close_socket_desc (&telnet_s);
+ return;
+ }
+ }
+
+ /* Initialize the history library so that we can use the
+ previous command if user just press return. */
+ using_history ();
+
+ /* Register the socket with event handler. */
+ add_file_handler (client_fd, get_input_from_telnet, 0);
+
+ /* Remote connection is set up, show prompt. */
+ send (client_fd, mi_prompt, strlen (mi_prompt), 0);
+ }
+
+ /* In any case, close the listening socket as we only
+ allow one connection. */
+ close_socket_desc (&telnet_s);
+}
+
+static void
+close_socket_desc (int *fd)
+{
+ if((fd != NULL) && (*fd != -1) )
+ {
+ delete_file_handler (*fd);
+ close (*fd);
+ *fd = -1;
+ }
+}
+
+/* This is wrapper over recv. If recv returns some error then it closes
+ the socket. */
+
+static int
+receive (int *fd, void *buf, size_t n, int flags)
+{
+ int ret;
+
+ if (fd == NULL)
+ return -1;
+
+ ret = recv (*fd, buf, n, flags);
+ if (ret < 1)
+ {
+ close_socket_desc (fd);
+ }
+ return ret;
+}
+
+/* A utility function to send the output to client. */
+
+static void
+reply_msg (const char *msg)
+{
+ char *result = NULL;
+ long len;
+
+ if (client_fd != -1 && msg != NULL)
+ {
+ /* flush additional output if any */
+ if (str_file)
+ {
+ gdb_flush (str_file);
+ result = ui_file_xstrdup (str_file, &len);
+
+ send (client_fd, result, len, 0);
+
+ xfree (result);
+ ui_file_rewind (str_file);
+ }
+ send (client_fd, msg, strlen (msg), 0);
+ }
+}
+
+/* Sends the reply to remote client with extra '\r' character
+ for every '\n' in the msg so that it will show up correctly on
+ telnet console. */
+static void
+reply_msg_with_carriage_return (const char *msg)
+{
+ int len;
+ char data;
+ int i =0;
+
+ if (client_fd != -1 && msg != NULL)
+ {
+ len = strlen (msg);
+
+ for(i=0; i < len; i++)
+ {
+ send (client_fd, &msg[i], 1, 0);
+
+ if(msg[i] == '\n')
+ {
+ data = '\r';
+ send (client_fd, &data, 1, 0);
+ }
+ }
+ }
+}
+
+/* Get a line from the client and put the terminating NULL
+ at correct place. */
+
+static int
+receive_line (int *fd, char* buf, int buf_size)
+{
+ int size;
+ int empty_string = 1;
+ int i;
+
+ /* Read a line from the socket. */
+ size = receive (fd, buf, (buf_size - 1), 0);
+
+ if (size <= 0)
+ return size;
+
+ /* Put a string terminating NULL character after the first character
+ that is not \n or \r. */
+ for (i = (size - 1); i >=0; i--)
+ {
+ if ( (buf[i] != '\n') && (buf[i] != '\r') )
+ {
+ buf[i + 1] = '\0';
+ empty_string = 0;
+ break;
+ }
+ }
+
+ /* The client did not send anything except \n and \r, so just send an
+ empty string to execute_telnet_command. */
+ if (empty_string == 1)
+ buf[0] = '\0';
+
+ return size;
+
+}
+
+/* This function is called when we need to take a yes or no answer
+ from the user. */
+
+static char
+get_answer_hook_line (int *fd)
+{
+ int size;
+ char cmd[MAX_CMD_LENGTH];
+
+ size = receive (fd, &cmd, MAX_CMD_LENGTH, 0);
+
+ if (size <= 0)
+ {
+ return 0; /* If error return 0 */
+ }
+ return cmd[0];
+}
+
+/* This function is called when GDB needs to get the list of
+ commands to run on breakpoints. See 'commands'. */
+
+static char*
+get_input_hook_line (int* fd, char* prompt)
+{
+ long len;
+ char buf[MAX_CMD_LENGTH + 1];
+
+ reply_msg (prompt);
+
+ len = receive_line (&client_fd, buf, MAX_CMD_LENGTH + 1);
+
+ if (len <= 0)
+ return NULL;
+
+ return xstrdup (buf);
+}
+
+/* This function is called when data is available on the socket to read. */
+
+static void
+data_available_hook_line (int *fd)
+{
+ int size;
+ unsigned char buf[MAX_CMD_LENGTH+1];
+
+ size = receive_line (&client_fd, buf, MAX_CMD_LENGTH + 1);
+
+ if (size <= 0)
+ return;
+
+ execute_telnet_command (buf);
+
+ /* Print the prompt. */
+ send (*fd, mi_prompt, strlen (mi_prompt), 0);
+}
+
+/* This function setup the hooks that are called at various points.
+ The mode parameter gives us flexibility to support other modes in
+ future. */
+
+static void
+setup_hooks (int mode)
+{
+ /* Currently, we are supporting only one mode where we get the
+ whole line from the client. So the _line in the end. We can
+ , in future, support getting input character by character. The
+ hook mechanism provides us the flexibility. */
+ if (mode == 0)
+ {
+ cmd_start_hook = NULL;
+
+ cmd_end_hook = NULL;
+
+ init_mode_hook = NULL;
+
+ data_available_hook = data_available_hook_line;
+
+ stop_service_hook = NULL;
+
+ get_answer_hook = get_answer_hook_line;
+
+ get_input_hook = get_input_hook_line;
+ }
+}
+
Index: gdb/testsuite/gdb.mi/mi-telnet.c
===================================================================
RCS file: gdb/testsuite/gdb.mi/mi-telnet.c
diff -N gdb/testsuite/gdb.mi/mi-telnet.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.mi/mi-telnet.c 12 Jul 2012 10:35:17 -0000
@@ -0,0 +1,73 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2012 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <netdb.h>
+#include <string.h>
+
+int test_connect(int port)
+{
+ struct sockaddr_in client;
+ struct hostent *hp;
+ int telnet_s;
+ int result = 0;
+ int err;
+
+ if ((hp = gethostbyname("localhost")) != 0)
+ {
+ memset ((char *)&client, 0, sizeof (client));
+ client.sin_family = AF_INET;
+ client.sin_port = htons (port);
+ client.sin_addr.s_addr = ((struct in_addr *)(hp->h_addr))->s_addr;
+
+ telnet_s = socket (AF_INET, SOCK_STREAM, IPPROTO_TCP);
+ if (telnet_s != -1)
+ {
+ /* connect to PORT on HOST */
+ err = connect(telnet_s, (struct sockaddr *) &client, sizeof(client));
+ if (err == -1)
+ {
+ result = 0; /* Connect-fail-breakpoint */
+ }
+ else
+ {
+ result = 1; /* Connect-succeed-breakpoint */
+ }
+ close(telnet_s);
+ }
+ }
+ return result;
+}
+
+
+int main(int argc, char* argv[])
+{
+ int port;
+ int result = 1;
+
+ if (argc == 2)
+ {
+ port = atoi (argv[1]);
+
+ if (port > 0)
+ {
+ result = test_connect (port);
+ }
+ }
+ return result; /* program-end-breakpoint */
+}
Index: gdb/testsuite/gdb.mi/mi-telnet.exp
===================================================================
RCS file: gdb/testsuite/gdb.mi/mi-telnet.exp
diff -N gdb/testsuite/gdb.mi/mi-telnet.exp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.mi/mi-telnet.exp 12 Jul 2012 10:35:17 -0000
@@ -0,0 +1,109 @@
+# Copyright 1999-2002, 2004-2005, 2007-2012 Free Software Foundation,
+# Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test -start-telnet-service and -stop-telnet-service comands.
+#
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi2"
+set PORT1 8005
+set PORT2 8006
+
+# Executes the test for telnet service.
+# port is the port on which telnet service will listen.
+# bp_line is the line in test C file where we want to hit the
+# breakpoint.
+# message is the message to print for this test.
+proc connect_test {port bp_line message} {
+ global srcfile
+ global connect_succeed_break
+ global connect_fail_break
+ global program_end_breakpoint
+
+ # set the argument of the test program
+ mi_gdb_test "-interpreter-exec console \"set args $port\"" \
+ {\^done} \
+ "-interpreter-exec console \"set args $port\""
+
+ mi_create_breakpoint "${srcfile}:${connect_succeed_break}" \
+ 2 .* test_connect ".*${srcfile}" ${connect_succeed_break} .* "Connection Succeed breakpoint."
+
+ mi_create_breakpoint "${srcfile}:${connect_fail_break}" \
+ 3 .* test_connect ".*${srcfile}" ${connect_fail_break} .* "Connection fail breakpoint."
+
+ mi_create_breakpoint "${srcfile}:${program_end_breakpoint}" \
+ 4 .* main ".*${srcfile}" ${program_end_breakpoint} .* "Program exit."
+
+ mi_run_cmd
+
+ mi_expect_stop "breakpoint-hit" "test_connect" ".*" ".*mi-telnet.c" "$bp_line" \
+ {"" "disp=\"keep\"" } $message
+}
+
+gdb_exit
+if [mi_gdb_start] {
+ continue
+}
+
+set testfile "mi-telnet"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/mi-telnet
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+ untested "could not compile test program"
+ return -1
+}
+
+mi_delete_breakpoints
+mi_gdb_reinitialize_dir $srcdir/$subdir
+mi_gdb_load ${binfile}
+
+mi_run_to_main
+
+set connect_succeed_break [gdb_get_line_number "Connect-succeed-breakpoint"]
+set connect_fail_break [gdb_get_line_number "Connect-fail-breakpoint"]
+set program_end_breakpoint [gdb_get_line_number "program-end-breakpoint"]
+
+# Send the command to start the telnet service
+mi_gdb_test "-start-telnet-service $PORT1" "\\\^done" "Start Telnet Service"
+
+# Check if test program can connect to the port.
+connect_test $PORT1 $connect_succeed_break "Client can connect when port is open."
+
+# Re-run the same test but after closing the telnet service. Test if client
+# can connect.
+mi_gdb_exit
+if [mi_gdb_start] {
+ continue
+}
+
+mi_delete_breakpoints
+mi_gdb_reinitialize_dir $srcdir/$subdir
+mi_gdb_load ${binfile}
+
+mi_run_to_main
+
+# Open the telnet service and then close it.
+mi_gdb_test "-start-telnet-service $PORT2" "\\\^done" "Start Telnet Service"
+
+mi_gdb_test "-stop-telnet-service" "\\\^done" "Stop Telnet Service"
+
+# Client program should not be able to connect now.
+connect_test $PORT2 $connect_fail_break "Client cannot connect when port is closed."
+
+mi_gdb_exit
+
+return 0
+
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [patch] MI telnet service
2012-07-12 11:21 [patch] MI telnet service Abid, Hafiz
@ 2012-07-12 13:19 ` Gary Benson
2012-07-12 14:37 ` Abid, Hafiz
2012-07-12 13:29 ` Eli Zaretskii
` (2 subsequent siblings)
3 siblings, 1 reply; 20+ messages in thread
From: Gary Benson @ 2012-07-12 13:19 UTC (permalink / raw)
To: Abid, Hafiz; +Cc: gdb-patches
Hi,
This isn't a formal review, just some formatting issues:
Abid, Hafiz wrote:
> +/* Accept client connection and register it
> + with event_loop to get data from telnet client. */
> +static void telnet_accept_handler(int err, gdb_client_data);
Missing space before '('.
> +/* Hooks which are called at various points during telnet
> + command processing. */
Need double space before '*/'
> + memset ((char *)&serv, 0, sizeof (serv));
Missing space after cast.
> + if (bind (telnet_s, (const struct sockaddr *)&serv, sizeof (serv)) == -1)
Missing space after cast.
> + error (_("Error: port %d can't be bind"), port);
I'm not sure about this error message. Maybe it should be "bound", or
maybe the whole error should say something else.
> + if(str_file != NULL)
Missing space before '('.
> + if(get_answer_hook)
Missing space before '('.
> + /* Check the first letter in the answer. */
> + switch (cmd)
> + {
> + case 'y':
> + case 'Y':
> + retval = 1;
> + break;
> +
> + case 'n':
> + case 'N':
> + default:
> + retval = 0;
> + break;
> + }
> + }
"break"s should line up with the lines above.
> + /* If this is a valid command then add it to history. */
> + if (*line != '\0' )
Extra space before ')'.
> + if ( (hist != NULL) && (hist->line != NULL) && (*hist->line != '\0') )
Extra spaces between '( (' and ') )'.
> + /* figure out current interpreter */
> + if (current_interp_named_p (INTERP_MI))
> + {
> + old_interp = interp_lookup (INTERP_MI);
> + }
There are a few places in this file where a single line following an
'if' is enclosed in '{}'. I don't know if this is ok or not, but it
looks funny to me.
> + reply_msg ("Error: unexpectedly failed to use CLI the interpreter\r\n");
This line is probably too long.
> + s = accept (telnet_s, (struct sockaddr *)&client, &len);
Missing space after cast.
> + if((fd != NULL) && (*fd != -1) )
Missing space before '(' and extra space before ')'. The inner
parentheses are superfluous and should probably be removed.
> +/* Sends the reply to remote client with extra '\r' character
> + for every '\n' in the msg so that it will show up correctly on
> + telnet console. */
> +static void
> +reply_msg_with_carriage_return (const char *msg)
Missing newline between comment and function.
> + for(i=0; i < len; i++)
Missing space before '(' and around '='.
> + if(msg[i] == '\n')
Missing space before '('.
> + for (i = (size - 1); i >=0; i--)
Extra parens, and missing space after '>='.
> + if ( (buf[i] != '\n') && (buf[i] != '\r') )
Extra parens and space. I would write this line like this:
if (buf[i] != '\n' && buf[i] != '\r')
> +static char*
Missing space before '*'.
> +/* This function is called when data is available on the socket to read. */
Need double space before '*/'
> +/* This function setup the hooks that are called at various points.
> + The mode parameter gives us flexibility to support other modes in
> + future. */
Need double space before '*/'
> diff -N gdb/testsuite/gdb.mi/mi-telnet.c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ gdb/testsuite/gdb.mi/mi-telnet.c 12 Jul 2012 10:35:17 -0000
The indentation in this file is completely wrong. There's also
a lot of missing space before '(' for function calls and missing
space after casts.
Cheers,
Gary
--
http://gbenson.net/
^ permalink raw reply [flat|nested] 20+ messages in thread* RE: [patch] MI telnet service
2012-07-12 13:19 ` Gary Benson
@ 2012-07-12 14:37 ` Abid, Hafiz
2012-07-12 15:08 ` Gary Benson
2012-07-15 8:13 ` Jan Kratochvil
0 siblings, 2 replies; 20+ messages in thread
From: Abid, Hafiz @ 2012-07-12 14:37 UTC (permalink / raw)
To: Gary Benson; +Cc: gdb-patches
> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Gary Benson
> Sent: Thursday, July 12, 2012 2:20 PM
> To: Abid, Hafiz
> Cc: gdb-patches@sourceware.org
> Subject: Re: [patch] MI telnet service
>
> Hi,
>
> This isn't a formal review, just some formatting issues:
>
> Abid, Hafiz wrote:
> > +/* Accept client connection and register it
> > + with event_loop to get data from telnet client. */
> > +static void telnet_accept_handler(int err, gdb_client_data);
>
> Missing space before '('.
>
> > +/* Hooks which are called at various points during telnet
> > + command processing. */
>
> Need double space before '*/'
>
> > + memset ((char *)&serv, 0, sizeof (serv));
>
> Missing space after cast.
>
> > + if (bind (telnet_s, (const struct sockaddr *)&serv, sizeof (serv))
> == -1)
>
> Missing space after cast.
>
> > + error (_("Error: port %d can't be bind"), port);
>
> I'm not sure about this error message. Maybe it should be "bound", or
> maybe the whole error should say something else.
>
> > + if(str_file != NULL)
>
> Missing space before '('.
>
> > + if(get_answer_hook)
>
> Missing space before '('.
>
> > + /* Check the first letter in the answer. */
> > + switch (cmd)
> > + {
> > + case 'y':
> > + case 'Y':
> > + retval = 1;
> > + break;
> > +
> > + case 'n':
> > + case 'N':
> > + default:
> > + retval = 0;
> > + break;
> > + }
> > + }
>
> "break"s should line up with the lines above.
>
> > + /* If this is a valid command then add it to history. */
> > + if (*line != '\0' )
>
> Extra space before ')'.
>
> > + if ( (hist != NULL) && (hist->line != NULL) && (*hist->line !=
> '\0') )
>
> Extra spaces between '( (' and ') )'.
>
> > + /* figure out current interpreter */
> > + if (current_interp_named_p (INTERP_MI))
> > + {
> > + old_interp = interp_lookup (INTERP_MI);
> > + }
>
> There are a few places in this file where a single line following an
> 'if' is enclosed in '{}'. I don't know if this is ok or not, but it
> looks funny to me.
>
> > + reply_msg ("Error: unexpectedly failed to use CLI the
> interpreter\r\n");
>
> This line is probably too long.
>
> > + s = accept (telnet_s, (struct sockaddr *)&client, &len);
>
> Missing space after cast.
>
> > + if((fd != NULL) && (*fd != -1) )
>
> Missing space before '(' and extra space before ')'. The inner
> parentheses are superfluous and should probably be removed.
>
> > +/* Sends the reply to remote client with extra '\r' character
> > + for every '\n' in the msg so that it will show up correctly on
> > + telnet console. */
> > +static void
> > +reply_msg_with_carriage_return (const char *msg)
>
> Missing newline between comment and function.
>
> > + for(i=0; i < len; i++)
>
> Missing space before '(' and around '='.
>
> > + if(msg[i] == '\n')
>
> Missing space before '('.
>
> > + for (i = (size - 1); i >=0; i--)
>
> Extra parens, and missing space after '>='.
>
> > + if ( (buf[i] != '\n') && (buf[i] != '\r') )
>
> Extra parens and space. I would write this line like this:
>
> if (buf[i] != '\n' && buf[i] != '\r')
>
> > +static char*
>
> Missing space before '*'.
>
> > +/* This function is called when data is available on the socket to
> read. */
>
> Need double space before '*/'
>
> > +/* This function setup the hooks that are called at various points.
> > + The mode parameter gives us flexibility to support other modes in
> > + future. */
>
> Need double space before '*/'
>
> > diff -N gdb/testsuite/gdb.mi/mi-telnet.c
> > --- /dev/null 1 Jan 1970 00:00:00 -0000
> > +++ gdb/testsuite/gdb.mi/mi-telnet.c 12 Jul 2012 10:35:17 -0000
>
> The indentation in this file is completely wrong. There's also
> a lot of missing space before '(' for function calls and missing
> space after casts.
I thought we are not required to follow the convention in the test program source. Anyway, I will fix it with the rest. Thanks for reviewing.
>
> Cheers,
> Gary
>
> --
> http://gbenson.net/
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [patch] MI telnet service
2012-07-12 14:37 ` Abid, Hafiz
@ 2012-07-12 15:08 ` Gary Benson
2012-07-15 8:13 ` Jan Kratochvil
1 sibling, 0 replies; 20+ messages in thread
From: Gary Benson @ 2012-07-12 15:08 UTC (permalink / raw)
To: Abid, Hafiz; +Cc: gdb-patches
Abid, Hafiz wrote:
> Gary Benson wrote:
> > > diff -N gdb/testsuite/gdb.mi/mi-telnet.c
> > > --- /dev/null 1 Jan 1970 00:00:00 -0000
> > > +++ gdb/testsuite/gdb.mi/mi-telnet.c 12 Jul 2012 10:35:17 -0000
> >
> > The indentation in this file is completely wrong. There's also
> > a lot of missing space before '(' for function calls and missing
> > space after casts.
>
> I thought we are not required to follow the convention in the test
> program source.
Ah, possibly. Perhaps someone more familiar could comment?
> Thanks for reviewing.
No problem :)
Cheers,
Gary
--
http://gbenson.net/
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [patch] MI telnet service
2012-07-12 14:37 ` Abid, Hafiz
2012-07-12 15:08 ` Gary Benson
@ 2012-07-15 8:13 ` Jan Kratochvil
1 sibling, 0 replies; 20+ messages in thread
From: Jan Kratochvil @ 2012-07-15 8:13 UTC (permalink / raw)
To: Abid, Hafiz; +Cc: Gary Benson, gdb-patches
On Thu, 12 Jul 2012 16:36:43 +0200, Abid, Hafiz wrote:
> I thought we are not required to follow the convention in the test program
> source. Anyway, I will fix it with the rest. Thanks for reviewing.
Like it is not worth to for example fix indentation in the existing testcases.
But checking in a new testcase with very different formatting style does not
make it easily fixable/update-able later. At least run indent with the
options from GNU Coding Standard:
-nbad -bap -nbc -bbo -bl -bli2 -bls -ncdb -nce -cp1 -cs -di2 -ndj -nfc1 -nfca -hnl -i2 -ip5 -lp -pcs -psl -nsc -nsob
Thanks,
Jan
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [patch] MI telnet service
2012-07-12 11:21 [patch] MI telnet service Abid, Hafiz
2012-07-12 13:19 ` Gary Benson
@ 2012-07-12 13:29 ` Eli Zaretskii
2012-07-12 14:45 ` Abid, Hafiz
2012-07-17 17:53 ` Vladimir Prus
2012-07-25 13:02 ` Jan Kratochvil
3 siblings, 1 reply; 20+ messages in thread
From: Eli Zaretskii @ 2012-07-12 13:29 UTC (permalink / raw)
To: Abid, Hafiz; +Cc: gdb-patches
> From: "Abid, Hafiz" <Hafiz_Abid@mentor.com>
> Date: Thu, 12 Jul 2012 11:20:36 +0000
>
> This patch provides implementation of telnet service. This is based on initial work by Grigory Tolstolytkin.(http://sourceware.org/ml/gdb-patches/2011-11/msg00466.html)
Thanks. A few comments about the documentation part:
> +Start a telnet service on the given port. After the service is started,
^^
Two spaces between sentences, please (here and elsewhere, including in
comments to C code).
> +user can connect to the gdb via telnet and execute CLI commands in
^^^
"@value{GDBN}" instead of a literal "gdb".
> +new one. For telnet service to work, @value{GDBN}
> +should be configured with --enable-gdbmitel=yes.
Use @option for options, like this: @samp{--enable-gdbmitel=yes}.
Btw, why does it make sense to require a configure-time option for
this feature? why not include it by default?
^ permalink raw reply [flat|nested] 20+ messages in thread* RE: [patch] MI telnet service
2012-07-12 13:29 ` Eli Zaretskii
@ 2012-07-12 14:45 ` Abid, Hafiz
2012-07-14 19:37 ` Marc Khouzam
0 siblings, 1 reply; 20+ messages in thread
From: Abid, Hafiz @ 2012-07-12 14:45 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Eli Zaretskii
> Sent: Thursday, July 12, 2012 2:29 PM
> To: Abid, Hafiz
> Cc: gdb-patches@sourceware.org
> Subject: Re: [patch] MI telnet service
>
> > From: "Abid, Hafiz" <Hafiz_Abid@mentor.com>
> > Date: Thu, 12 Jul 2012 11:20:36 +0000
> >
> > This patch provides implementation of telnet service. This is based
> on initial work by Grigory Tolstolytkin.(http://sourceware.org/ml/gdb-
> patches/2011-11/msg00466.html)
>
> Thanks. A few comments about the documentation part:
>
> > +Start a telnet service on the given port. After the service is
> started,
> ^^
> Two spaces between sentences, please (here and elsewhere, including in
> comments to C code).
>
> > +user can connect to the gdb via telnet and execute CLI commands in
> ^^^
> "@value{GDBN}" instead of a literal "gdb".
>
> > +new one. For telnet service to work, @value{GDBN}
> > +should be configured with --enable-gdbmitel=yes.
>
> Use @option for options, like this: @samp{--enable-gdbmitel=yes}.
>
> Btw, why does it make sense to require a configure-time option for
> this feature? why not include it by default?
It just felt a bit safer to keep it optional. If community thinks that it is useful feature then I see no problem in making it available by default.
Thanks,
Abid
^ permalink raw reply [flat|nested] 20+ messages in thread* RE: [patch] MI telnet service
2012-07-12 14:45 ` Abid, Hafiz
@ 2012-07-14 19:37 ` Marc Khouzam
2012-07-24 16:17 ` Abid, Hafiz
0 siblings, 1 reply; 20+ messages in thread
From: Marc Khouzam @ 2012-07-14 19:37 UTC (permalink / raw)
To: Abid, Hafiz, Eli Zaretskii; +Cc: gdb-patches
> > -----Original Message-----
> > From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> > owner@sourceware.org] On Behalf Of Eli Zaretskii
> > Sent: Thursday, July 12, 2012 2:29 PM
> > To: Abid, Hafiz
> > Cc: gdb-patches@sourceware.org
> > Subject: Re: [patch] MI telnet service
> >
> > > From: "Abid, Hafiz" <Hafiz_Abid@mentor.com>
> > > Date: Thu, 12 Jul 2012 11:20:36 +0000
> > >
> > > This patch provides implementation of telnet service. This is based
> > on initial work by Grigory Tolstolytkin.(http://sourceware.org/ml/gdb-
> > patches/2011-11/msg00466.html)
> >
> > Thanks. A few comments about the documentation part:
> >
> > > +Start a telnet service on the given port. After the service is
> > started,
> > ^^
> > Two spaces between sentences, please (here and elsewhere, including in
> > comments to C code).
> >
> > > +user can connect to the gdb via telnet and execute CLI commands in
> > ^^^
> > "@value{GDBN}" instead of a literal "gdb".
> >
> > > +new one. For telnet service to work, @value{GDBN}
> > > +should be configured with --enable-gdbmitel=yes.
> >
> > Use @option for options, like this: @samp{--enable-gdbmitel=yes}.
> >
> > Btw, why does it make sense to require a configure-time option for
> > this feature? why not include it by default?
>
> It just felt a bit safer to keep it optional. If community thinks that
> it is useful feature then I see no problem in making it available by default.
I think it makes sense to have a way to turn off this feature. Some people may
think it is a security issue to have this port open and wish to keep the feature
completely disabled. However, if Eclipse is to make use of this feature, it
has to be enabled in GDB, and it would complicate things greatly if we had to
ask the user to re-compile GDB to enable this setting. It would be much nicer
if it worked directly in the GDB distributed with their linux box.
Therefore, I hope we can have this setting enabled by default.
I tried out the patch with and without Eclipse and it looks quite good!
Here are some issues I found:
- If I set a tty (-inferior-tty-set), GDB exits (or crashes?) when sending an
empty command (just pressing enter) over telnet. Eclipse uses a tty for the
inferiors.
- For every command sent over telnet, there is a printout in the orinal GDB
saying "Switching to interpreter "console"." Not a big deal, but maybe
something to remove.
- starting the telnet service, connecting to it, and then stopping the
service (-stop-telnet-service), does no seem to clean up the port propertly
and I see:
^error,msg="Error: port 9996 can't be bind"
- If I start GDB in CLI mode, and try to use the telnet service, GDB segfaults
after the first command is sent over telnet. I know this service is not
currently meant to be used with the CLI mode, but it should not crash :)
I personally think this would be cool to have even for CLI mode, but if it
won't be for now, maybe the telnet service should be disabled in that case.
I realize this is a first and important step, but it will be even nicer
when the telnet service supports tab-completion and command-history.
This is very exciting! Thanks!
Marc
^ permalink raw reply [flat|nested] 20+ messages in thread* RE: [patch] MI telnet service
2012-07-14 19:37 ` Marc Khouzam
@ 2012-07-24 16:17 ` Abid, Hafiz
2012-07-24 19:06 ` Marc Khouzam
0 siblings, 1 reply; 20+ messages in thread
From: Abid, Hafiz @ 2012-07-24 16:17 UTC (permalink / raw)
To: Marc Khouzam; +Cc: gdb-patches, Eli Zaretskii
> -----Original Message-----
> From: Marc Khouzam [mailto:marc.khouzam@ericsson.com]
> Sent: Saturday, July 14, 2012 8:37 PM
> To: Abid, Hafiz; Eli Zaretskii
> Cc: gdb-patches@sourceware.org
> Subject: RE: [patch] MI telnet service
>
> > > -----Original Message-----
> > > From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> > > owner@sourceware.org] On Behalf Of Eli Zaretskii
> > > Sent: Thursday, July 12, 2012 2:29 PM
> > > To: Abid, Hafiz
> > > Cc: gdb-patches@sourceware.org
> > > Subject: Re: [patch] MI telnet service
> > >
> > > > From: "Abid, Hafiz" <Hafiz_Abid@mentor.com>
> > > > Date: Thu, 12 Jul 2012 11:20:36 +0000
> > > >
> > > > This patch provides implementation of telnet service. This is
> based
> > > on initial work by Grigory
> Tolstolytkin.(http://sourceware.org/ml/gdb-
> > > patches/2011-11/msg00466.html)
> > >
> > > Thanks. A few comments about the documentation part:
> > >
> > > > +Start a telnet service on the given port. After the service is
> > > started,
> > > ^^
> > > Two spaces between sentences, please (here and elsewhere, including
> in
> > > comments to C code).
> > >
> > > > +user can connect to the gdb via telnet and execute CLI commands
> in
> > > ^^^
> > > "@value{GDBN}" instead of a literal "gdb".
> > >
> > > > +new one. For telnet service to work, @value{GDBN}
> > > > +should be configured with --enable-gdbmitel=yes.
> > >
> > > Use @option for options, like this: @samp{--enable-gdbmitel=yes}.
> > >
> > > Btw, why does it make sense to require a configure-time option for
> > > this feature? why not include it by default?
> >
> > It just felt a bit safer to keep it optional. If community thinks
> that
> > it is useful feature then I see no problem in making it available by
> default.
>
> I think it makes sense to have a way to turn off this feature. Some
> people may
> think it is a security issue to have this port open and wish to keep
> the feature
> completely disabled. However, if Eclipse is to make use of this
> feature, it
> has to be enabled in GDB, and it would complicate things greatly if we
> had to
> ask the user to re-compile GDB to enable this setting. It would be
> much nicer
> if it worked directly in the GDB distributed with their linux box.
> Therefore, I hope we can have this setting enabled by default.
>
> I tried out the patch with and without Eclipse and it looks quite good!
> Here are some issues I found:
>
> - If I set a tty (-inferior-tty-set), GDB exits (or crashes?) when
> sending an
> empty command (just pressing enter) over telnet. Eclipse uses a tty
> for the
> inferiors.
Can you kindly list the steps to reproduce this crash. I tried but it worked ok for me.
>
> - For every command sent over telnet, there is a printout in the orinal
> GDB
> saying "Switching to interpreter "console"." Not a big deal, but maybe
> something to remove.
>
> - starting the telnet service, connecting to it, and then stopping the
> service (-stop-telnet-service), does no seem to clean up the port
> propertly
> and I see:
> ^error,msg="Error: port 9996 can't be bind"
>
> - If I start GDB in CLI mode, and try to use the telnet service, GDB
> segfaults
> after the first command is sent over telnet. I know this service is
> not
> currently meant to be used with the CLI mode, but it should not crash
> :)
> I personally think this would be cool to have even for CLI mode, but if
> it
> won't be for now, maybe the telnet service should be disabled in that
> case.
>
> I realize this is a first and important step, but it will be even nicer
> when the telnet service supports tab-completion and command-history.
>
> This is very exciting! Thanks!
>
> Marc
Thanks for taking time to test.
Regards,
Abid
^ permalink raw reply [flat|nested] 20+ messages in thread* RE: [patch] MI telnet service
2012-07-24 16:17 ` Abid, Hafiz
@ 2012-07-24 19:06 ` Marc Khouzam
2012-07-24 20:15 ` Marc Khouzam
0 siblings, 1 reply; 20+ messages in thread
From: Marc Khouzam @ 2012-07-24 19:06 UTC (permalink / raw)
To: 'Abid, Hafiz'
Cc: 'gdb-patches@sourceware.org', 'Eli Zaretskii'
> > Here are some issues I found:
> >
> > - If I set a tty (-inferior-tty-set), GDB exits (or crashes?) when
> > sending an
> > empty command (just pressing enter) over telnet. Eclipse uses a tty
> > for the
> > inferiors.
> Can you kindly list the steps to reproduce this crash. I
> tried but it worked ok for me.
It happens all the time with Eclipse. I thought I had also
reproduced it on the command line but today I just can get it to
happen anymore directly in GDB. But I also can't stop it from
happening in Eclipse :)
I reproduced the _exact_ same MI session outside of eclipse
as the one that crashes, and it still doesn't happen.
I'll have to figure out what is different.
You could try it with Eclipse and see if it happens to you also.
Just launch a debug session and start the telnet service
from the eclipse gdb console, then telnet in and press enter.
While trying to reproduce this, I ran into another problem
that I could reproduce outside of Eclispe.
In async-mode, gdb seems to go into a tight-loop when resuming
execution from the telnet session (excerpts of the session below):
> ./gdb/gdb -i mi <any program>
~"GNU gdb (GDB) 7.5.50.20120724-cvs\n"
(gdb) -gdb-set target-async on
^done
(gdb) start
&"start\n"
~"Temporary breakpoint 1 at 0x804850d: file loopfirst.cc, line 5.\n"
[...]
*stopped,reason="breakpoint-hit",disp="del",bkptno="1",frame={addr="0x0804850d",func="main",args=[],file="loopfirst.cc",fullname="/home/lmckhou/testing/loopfirst.cc",line="5"},thread-id="1",stopped-threads="all",core="0"
=breakpoint-deleted,id="1"
(gdb) -start-telnet-service 9999
^done
=> start a telnet session from another terminal and give the 'next' command
=> from the telnet session.
Switching to interpreter "console".
(gdb) ^running
*running,thread-id="all"
=> gdb grabs 100% of the CPU and must be killed with -9
> > - For every command sent over telnet, there is a printout
> in the orinal
> > GDB
> > saying "Switching to interpreter "console"." Not a big
> deal, but maybe
> > something to remove.
> >
> > - starting the telnet service, connecting to it, and then
> stopping the
> > service (-stop-telnet-service), does no seem to clean up the port
> > propertly
> > and I see:
> > ^error,msg="Error: port 9996 can't be bind"
> >
> > - If I start GDB in CLI mode, and try to use the telnet service, GDB
> > segfaults
> > after the first command is sent over telnet. I know this service is
> > not
> > currently meant to be used with the CLI mode, but it should
> not crash
> > :)
> > I personally think this would be cool to have even for CLI
> mode, but if
> > it
> > won't be for now, maybe the telnet service should be
> disabled in that
> > case.
> >
> > I realize this is a first and important step, but it will
> be even nicer
> > when the telnet service supports tab-completion and command-history.
> >
> > This is very exciting! Thanks!
> >
> > Marc
> Thanks for taking time to test.
>
> Regards,
> Abid
>
^ permalink raw reply [flat|nested] 20+ messages in thread* RE: [patch] MI telnet service
2012-07-24 19:06 ` Marc Khouzam
@ 2012-07-24 20:15 ` Marc Khouzam
0 siblings, 0 replies; 20+ messages in thread
From: Marc Khouzam @ 2012-07-24 20:15 UTC (permalink / raw)
To: 'Abid, Hafiz'
Cc: 'gdb-patches@sourceware.org', 'Eli Zaretskii'
> -----Original Message-----
> From: gdb-patches-owner@sourceware.org
> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Marc Khouzam
> Sent: Tuesday, July 24, 2012 3:06 PM
> To: 'Abid, Hafiz'
> Cc: 'gdb-patches@sourceware.org'; 'Eli Zaretskii'
> Subject: RE: [patch] MI telnet service
>
> > > Here are some issues I found:
> > >
> > > - If I set a tty (-inferior-tty-set), GDB exits (or crashes?) when
> > > sending an
> > > empty command (just pressing enter) over telnet. Eclipse
> uses a tty
> > > for the
> > > inferiors.
> > Can you kindly list the steps to reproduce this crash. I
> > tried but it worked ok for me.
>
> It happens all the time with Eclipse. I thought I had also
> reproduced it on the command line but today I just can get it to
> happen anymore directly in GDB. But I also can't stop it from
> happening in Eclipse :)
> I reproduced the _exact_ same MI session outside of eclipse
> as the one that crashes, and it still doesn't happen.
> I'll have to figure out what is different.
Ok, the problem is in the use of the readline history.
I couldn't reproduce it today because I wasn't starting
gdb where I had a ./.gdb_history file. But eclipse was
starting gdb in my $HOME which happened to have the
command 'q' as the last command in the history file.
So GDB was not crashing, it was just exiting when I
pressed <enter> in the telnet session. Nothing to do
with using a tty.
The problem is that the telnet service uses
'previous_history()' instead of 'current_history()'.
previous_history() modifies the history which is
not what you want here.
If you can change your patch to use current_history()
in the method execute_telnet_command()() of mi/mi-telnet.c,
it should fix things.
Thanks
> You could try it with Eclipse and see if it happens to you also.
> Just launch a debug session and start the telnet service
> from the eclipse gdb console, then telnet in and press enter.
>
> While trying to reproduce this, I ran into another problem
> that I could reproduce outside of Eclispe.
> In async-mode, gdb seems to go into a tight-loop when resuming
> execution from the telnet session (excerpts of the session below):
>
> > ./gdb/gdb -i mi <any program>
> ~"GNU gdb (GDB) 7.5.50.20120724-cvs\n"
> (gdb) -gdb-set target-async on
> ^done
> (gdb) start
> &"start\n"
> ~"Temporary breakpoint 1 at 0x804850d: file loopfirst.cc, line 5.\n"
> [...]
> *stopped,reason="breakpoint-hit",disp="del",bkptno="1",frame={
addr="0x0804850d",func="main",args=[],file="loopfirst.cc",fullna> me="/home/lmckhou/testing/loopfirst.cc",line="5"},thread-id="1
",stopped-threads="all",core="0"
> =breakpoint-deleted,id="1"
> (gdb) -start-telnet-service 9999
> ^done
>
> => start a telnet session from another terminal and give the
> 'next' command
> => from the telnet session.
>
> Switching to interpreter "console".
> (gdb) ^running
> *running,thread-id="all"
>
> => gdb grabs 100% of the CPU and must be killed with -9
>
>
> > > - For every command sent over telnet, there is a printout
> > in the orinal
> > > GDB
> > > saying "Switching to interpreter "console"." Not a big
> > deal, but maybe
> > > something to remove.
> > >
> > > - starting the telnet service, connecting to it, and then
> > stopping the
> > > service (-stop-telnet-service), does no seem to clean up the port
> > > propertly
> > > and I see:
> > > ^error,msg="Error: port 9996 can't be bind"
> > >
> > > - If I start GDB in CLI mode, and try to use the telnet
> service, GDB
> > > segfaults
> > > after the first command is sent over telnet. I know this
> service is
> > > not
> > > currently meant to be used with the CLI mode, but it should
> > not crash
> > > :)
> > > I personally think this would be cool to have even for CLI
> > mode, but if
> > > it
> > > won't be for now, maybe the telnet service should be
> > disabled in that
> > > case.
> > >
> > > I realize this is a first and important step, but it will
> > be even nicer
> > > when the telnet service supports tab-completion and
> command-history.
> > >
> > > This is very exciting! Thanks!
> > >
> > > Marc
> > Thanks for taking time to test.
> >
> > Regards,
> > Abid
> >
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [patch] MI telnet service
2012-07-12 11:21 [patch] MI telnet service Abid, Hafiz
2012-07-12 13:19 ` Gary Benson
2012-07-12 13:29 ` Eli Zaretskii
@ 2012-07-17 17:53 ` Vladimir Prus
2012-07-25 13:02 ` Jan Kratochvil
3 siblings, 0 replies; 20+ messages in thread
From: Vladimir Prus @ 2012-07-17 17:53 UTC (permalink / raw)
To: gdb-patches
On 12.07.2012 15:20, Abid, Hafiz wrote:
> Hi,
> This patch provides implementation of telnet service. This is based on initial work by Grigory Tolstolytkin.(http://sourceware.org/ml/gdb-patches/2011-11/msg00466.html)
>
> This service can accept and execute CLI commands from the remote user. It can be started by MI command "-start-telnet-service [port]". After the service is running, user can connect to the GDB via telnet and execute CLI commands in parallel with existing MI interface. This service can be stopped by another MI command "-stop-telnet-service". For these commands to work, GDB has to be configured with --enable-gdbmitel=yes.
>
> This service can be useful, for example, when GDB is running as a backend of an IDE. Testing infrastructure can run the tests by sending commands through socket. In case of an error, a user can debug the problem in the IDE.
>
> The test case that I added only checks that port is opened after the service is started. I am looking to improve it and would appreciate if the reviewers can help me with some ideas.
>
> How does it all look?
Hi Abid,
it seems to me that the essential part of this patch does not have much to do with MI proper, and therefore it should live outside of 'mi'
subdirectory.
> * mi/mi-cmds.c (mi_cmds): Add -start-telnet-service
> and stop-telnet-service MI commands.
> * mi/mi-cmds.h (mi_cmd_start_telnet_service): Add declaration.
> (mi_cmd_stop_telnet_service): Add declaration.
These changes, well, all 8 lines, look OK.
> * mi/mi-telnet.c: New file.
Beyond the suggestion that this code not be put under 'mi' directory, I have no say about the idea or implementation.
Thanks,
Volodya
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [patch] MI telnet service
2012-07-12 11:21 [patch] MI telnet service Abid, Hafiz
` (2 preceding siblings ...)
2012-07-17 17:53 ` Vladimir Prus
@ 2012-07-25 13:02 ` Jan Kratochvil
2012-07-25 14:12 ` Marc Khouzam
2012-07-25 14:16 ` Abid, Hafiz
3 siblings, 2 replies; 20+ messages in thread
From: Jan Kratochvil @ 2012-07-25 13:02 UTC (permalink / raw)
To: Abid, Hafiz; +Cc: gdb-patches
Hi Abid,
the testcase is mixing host vs. target execution. You connect to the port
from the .c program but that can run on a different machine than the .exp
program is running. It would be better to connect to the port from the .exp
file, in fact you do not need any .c program there at all. Or if the test
would be done from .c file then you need to skip the test if [is_remote target].
(It could run if the target is remote but still "localhost" or "stdio" but
that is currently not supported in the GDB testsuite.)
"gdbmitel" is too cryptic, the option can be very long, with words separated
by dashes.
Have you talked to Eclipse maintainers why they do not provide separate
console? In fact they do, I find it more fits there. If communication with
Eclipse developers has failed GDB can implement it, but I find it more as
Eclipse workaround.
It does not support IPv6. GDB currently also does not (for connections to
gdbserver) but that is a bug, I tried to fix it, I hope to resurrect the patch
sometimes again:
[patch] IPv6 support for gdbserver
http://sourceware.org/ml/gdb-patches/2006-09/msg00192.html
http://sourceware.org/ml/gdb-patches/2006-10/msg00073.html
Opening the port INADDR_ANY should be possible but only with some additional
-start-telnet-service option, services usually open only with INADDR_LOOPBACK
by default. Still there is missing security on multi-user systems, this is one
of the reasons an interface through Eclipse would be more logical to me.
You do not print any errors, such as in:
+/* This is wrapper over recv. If recv returns some error then it closes
+ the socket. */
+static int
+receive (int *fd, void *buf, size_t n, int flags)
I believe it could be output as:
Node: GDB/MI Stream Records
`"&" STRING-OUTPUT'
The log stream contains debugging messages being produced by GDB's
internals.
I find it only as a last resort possibility now without readline support.
Could it support $TERM negotiation like telnetd supports and attach readline
to it?
Thanks,
Jan
^ permalink raw reply [flat|nested] 20+ messages in thread* RE: [patch] MI telnet service
2012-07-25 13:02 ` Jan Kratochvil
@ 2012-07-25 14:12 ` Marc Khouzam
2012-07-25 19:27 ` Jan Kratochvil
2012-07-25 14:16 ` Abid, Hafiz
1 sibling, 1 reply; 20+ messages in thread
From: Marc Khouzam @ 2012-07-25 14:12 UTC (permalink / raw)
To: 'Jan Kratochvil', 'Abid, Hafiz'
Cc: 'gdb-patches@sourceware.org'
> -----Original Message-----
> From: gdb-patches-owner@sourceware.org
> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Jan Kratochvil
> Sent: Wednesday, July 25, 2012 9:02 AM
> To: Abid, Hafiz
> Cc: gdb-patches@sourceware.org
> Subject: Re: [patch] MI telnet service
> Have you talked to Eclipse maintainers why they do not
> provide separate
> console? In fact they do, I find it more fits there. If
> communication with
> Eclipse developers has failed GDB can implement it, but I
> find it more as Eclipse workaround.
I understand your point-of-view. There are advantages to
having this feature in GDB instead of Eclipse though.
The final goal is to allow the eclipse user to interact with
GDB using the same command-line features as the standard
GDB CLI, and to keep Eclipse up-to-date with respect
to the changes made through this console. The latter point
is being addressed separately by adding more MI events.
This telnet patch attempts to make it easier for Eclipse
to give a full CLI to the user directly in Eclipse.
Conceptually one can argue that "interacting directly with GDB
should be handled by GDB itself" and not have go through another
layer pretending to be GDB. Furthermore, something feels wrong
and limiting about providing a CLI look & feel while going through
the MI channel used by Eclipse.
In detail, the features Eclipse needs are:
- Command-history: This could be managed in Eclipse, but would
require more work to achieve the same flexibility and richness
that GDB provides.
- Readline features: This is more complicated. We could implement
the same readline features directly in Eclipse (there is probably
a package we could use to get this working) but re-using GDB's
existing feature seems a simpler path to take. Also, and I'm
not sure of this, it may be even more effort to get this to
work for Windows also, while GDB provides all this already.
- Command-completion: This could be achieved using GDB's 'complete'
command. The look & feel of this feature would probably not be
exactly like in GDB which may be an annoyance to the user. And
respecting GDB preferences (if there are some that affect completion)
will require some extra handling.
- Repeat last command: I don't believe this can be done in Eclipse
without GDB's help, and I don't know of a way to ask GDB if a
command should be repeated or not.
- Prompt: GDB does some smart handling of the prompt, which would have
to be replicated in Eclipse (or at least a poor-man's version).
- Queries: Because Eclipse uses MI, GDB handles queries differently
and such queries are being automatically defaulted instead of
being presented to the user.
There are probably a couple of more that slip my mind right now.
We felt that the most efficient way to get this feature was to
re-use as much of GDB's features as possible, which explains why
this telnet approach is being proposed.
Beyond the Eclipse use-case, I personally feel that this
telnet feature adds value to GDB for other situations.
For example, a designer could remote connect to a running
GDB that was setup in the lab, or one could continue a debug
session after having gone home. Not world-changing, but
valuable nonetheless IMHO.
Your comments are most welcomed.
Thanks
Marc
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [patch] MI telnet service
2012-07-25 14:12 ` Marc Khouzam
@ 2012-07-25 19:27 ` Jan Kratochvil
2012-07-26 0:02 ` Stan Shebs
2012-08-03 11:36 ` Jan Kratochvil
0 siblings, 2 replies; 20+ messages in thread
From: Jan Kratochvil @ 2012-07-25 19:27 UTC (permalink / raw)
To: Marc Khouzam; +Cc: 'Abid, Hafiz', 'gdb-patches@sourceware.org'
On Wed, 25 Jul 2012 16:12:14 +0200, Marc Khouzam wrote:
> The final goal is to allow the eclipse user to interact with
> GDB using the same command-line features as the standard
> GDB CLI,
If it should be used somehow more regularly the TCP port needs to be
reconsidered. It is already problem with gdbserver fortunately now solved by
the stdio communication option. Otherwise there is problem with security on
multi-user systems, also unique port allocation (why I introduced --once to
gdbserver) etc.
> In detail, the features Eclipse needs are:
The current problem of GDB is that its CLI is not built on top of MI (*).
Coding anything new is needlessly complicated needlessly combining all the
modes of operation together. And instead of fixing that you go the exactly
opposite direction extending use of this CLI-and-MI mixed interface even more.
(*) MI as some RPC protocol, maybe some different and already established RPC
protocol like D-Bus/CORBA/etc. would be better, there was a reply here MI
clients are fine with MI:
http://sourceware.org/ml/gdb-patches/2011-08/msg00586.html
Such change from MI I find orthogonal to the changes discussed below.
There were some considerations to write thin CLI client on top of MI (or even
something new) just to make the internal handling of CLI vs. MI more sane.
Investment into another extension on top of the current code which does not
even provide readline functionality I do not find the right direction.
I do not think we need to keep the behavior exactly the same, minor
differences making the interface more systematic could be allowed.
> - Command-history: This could be managed in Eclipse, but would
> require more work to achieve the same flexibility and richness
> that GDB provides.
Normal readline using ~/.gdb_history should work instead. Omitting now about
custom 'set history filename' etc.
> - Readline features: This is more complicated. We could implement
> the same readline features directly in Eclipse (there is probably
> a package we could use to get this working) but re-using GDB's
> existing feature seems a simpler path to take. Also, and I'm
> not sure of this, it may be even more effort to get this to
> work for Windows also, while GDB provides all this already.
There are some Java GNU readline-like packages. Personally I would be more
interested in simple C/C++ thin client using GNU readline on top of the MI (or
other) interface.
> - Command-completion: This could be achieved using GDB's 'complete'
> command. The look & feel of this feature would probably not be
> exactly like in GDB which may be an annoyance to the user. And
> respecting GDB preferences (if there are some that affect completion)
> will require some extra handling.
The 'complete' command seems right for it.
> - Repeat last command: I don't believe this can be done in Eclipse
> without GDB's help, and I don't know of a way to ask GDB if a
> command should be repeated or not.
There could be additional flag sent in the result record (if MI is used).
> - Prompt: GDB does some smart handling of the prompt, which would have
> to be replicated in Eclipse (or at least a poor-man's version).
Yes...
> - Queries: Because Eclipse uses MI, GDB handles queries differently
> and such queries are being automatically defaulted instead of
> being presented to the user.
It could default to "no" returning the question as ^error result record
requesting some flag "yes" during next command execution.
> We felt that the most efficient way to get this feature was to
> re-use as much of GDB's features as possible, which explains why
> this telnet approach is being proposed.
Do you find the proposal above worth investigation? I find the missing
readline and target-async requirement complication enough of the proposed
telnet solution that the CLI-on-top-of-MI solution working initially only for
90% of commands could be equally good enough.
Commands the CLI-on-top-of-MI interface can send as:
-interpreter-exec console "FOO"
There is a question whether to translate known commands into proper existing
MI commands or not. If one would want to replace MI that part would get
dropped but replacing MI I find too farsighted now that maybe some extension
of '-interpreter-exec console' would be more easy now.
> Beyond the Eclipse use-case, I personally feel that this
> telnet feature adds value to GDB for other situations.
> For example, a designer could remote connect to a running
> GDB that was setup in the lab, or one could continue a debug
> session after having gone home.
Sorry but 'screen' (or 'vnc' for Eclip=se) serves this purpose better.
Thanks,
Jan
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [patch] MI telnet service
2012-07-25 19:27 ` Jan Kratochvil
@ 2012-07-26 0:02 ` Stan Shebs
2012-07-26 11:51 ` Jan Kratochvil
2012-08-03 11:36 ` Jan Kratochvil
1 sibling, 1 reply; 20+ messages in thread
From: Stan Shebs @ 2012-07-26 0:02 UTC (permalink / raw)
To: gdb-patches
On 7/25/12 12:26 PM, Jan Kratochvil wrote:
> On Wed, 25 Jul 2012 16:12:14 +0200, Marc Khouzam wrote:
>> The final goal is to allow the eclipse user to interact with
>> GDB using the same command-line features as the standard
>> GDB CLI,
> If it should be used somehow more regularly the TCP port needs to be
> reconsidered. It is already problem with gdbserver fortunately now solved by
> the stdio communication option. Otherwise there is problem with security on
> multi-user systems, also unique port allocation (why I introduced --once to
> gdbserver) etc.
Essentially what this change is doing is making GDB itself a little more
server-like. It's not an accident that interpreters and ui-files are
both objects; it's been a little surprising to me that nobody has yet
added the multiple-port capability to take advantage of that.
>> In detail, the features Eclipse needs are:
> The current problem of GDB is that its CLI is not built on top of MI (*).
Huh, I don't recall ever hearing that diagnosis before. It seems like
it would be incredibly hard to migrate the CLI to be an MI veneer; not
only is there interactive CLI behavior that MI has never had to worry
about (because it's, well, a machine interface), but MI has generally
limited itself to features that were known to be of use to a frontend.
> Coding anything new is needlessly complicated needlessly combining all the
> modes of operation together. And instead of fixing that you go the exactly
> opposite direction extending use of this CLI-and-MI mixed interface even more.
The patch is kind of small though; it's mostly enabling latent
capabilities, and not trying to rewrite any existing subsystems. The
alternatives you suggest seem rather complicated by comparison.
>
>> - Readline features: This is more complicated. We could implement
>> the same readline features directly in Eclipse (there is probably
>> a package we could use to get this working) but re-using GDB's
>> existing feature seems a simpler path to take. Also, and I'm
>> not sure of this, it may be even more effort to get this to
>> work for Windows also, while GDB provides all this already.
> There are some Java GNU readline-like packages. Personally I would be more
> interested in simple C/C++ thin client using GNU readline on top of the MI (or
> other) interface.
Maybe nobody else thinks this, but adding an additional program into the
Eclipse/GDB combination seems like a far more complicated approach, and
has its own reliability issues, such as when the thin client crashes or
hangs.
>> Beyond the Eclipse use-case, I personally feel that this
>> telnet feature adds value to GDB for other situations.
>> For example, a designer could remote connect to a running
>> GDB that was setup in the lab, or one could continue a debug
>> session after having gone home.
> Sorry but 'screen' (or 'vnc' for Eclip=se) serves this purpose better.
>
Not so much if it's a second person that is, say, running a diagnostic
script, and doesn't have any use for a mirror image of all the windows.
But to pull back and take a little broader view, in a system of software
components, I think GDB has a useful role as a server that can have
multiple frontends, telnet-like sessions, scripted drivers, and so
forth. Perhaps we should decide on whether the server concept is
plausible (maybe I'm the only one who thinks that way) or to be avoided,
and then go from there.
Stan
stan@codesourcery.com
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [patch] MI telnet service
2012-07-26 0:02 ` Stan Shebs
@ 2012-07-26 11:51 ` Jan Kratochvil
0 siblings, 0 replies; 20+ messages in thread
From: Jan Kratochvil @ 2012-07-26 11:51 UTC (permalink / raw)
To: Stan Shebs; +Cc: gdb-patches
On Thu, 26 Jul 2012 02:02:07 +0200, Stan Shebs wrote:
> Essentially what this change is doing is making GDB itself a little
> more server-like.
I find this design fine, I am not against it.
But from the implementation point of view it mixes together with the long-term
pain of mixed CLI and MI processing. The goal is to produced only raw data
(like MI does) by current GDB code and to have a completely different code
formatting the MI data as CLI (the way Eclipse formats the MI data as GUI)
------------------------------------------------------------------------------
If it is not clear why the CLI and MI processing mix is a pain I can
illustrate on (but there are many such cases in GDB)
http://sourceware.org/ml/gdb-patches/2011-09/msg00558.html
as the code handles 7 modes of "set print entry-values" setting (it has to).
But additionally to it the same code has to handle both proper readable output
in CLI mode and proper MI output where the MI output is done in two passes.
In reality one has to put all the combinations into the testsuite as otherwise
a minor edit will break some of the 21 modes each lines of code runs in.
------------------------------------------------------------------------------
Formally the patterns of separating of the code I have Googled out as:
http://en.wikipedia.org/wiki/Separation_of_concerns
specifically here proposing roughly current GDB to be 'model', its MI to be
'adapter' and new-CLI-on-top-of-MI client + Eclipse to be 'view':
http://en.wikipedia.org/wiki/Model-view-adapter
This way the interface of GDB will be only MI which can be easily multiplexed
into multiple views. One of the view to be still Eclipse and other views to be
each CLI (also easily featuring GNU readline).
> It's not an accident that interpreters and ui-files are both objects; it's
> been a little surprising to me that nobody has yet added the multiple-port
> capability to take advantage of that.
If there is some way how to implement multiple GNU readlines it may be
reconsidered but I find CLI without readline to be unusable.
> Huh, I don't recall ever hearing that diagnosis before. It seems
> like it would be incredibly hard to migrate the CLI to be an MI
> veneer;
See above.
> not only is there interactive CLI behavior
That's the problem, mixing UI with GDB logic. Also because usually different
types of developers maintain each kind of code, which is unfortunately
currently mixed together.
> that MI has never had to worry about (because it's, well, a machine
> interface), but MI has generally limited itself to features that were known
> to be of use to a frontend.
Yes, MI would need to be extended many times so that the CLI-on-top-of-MI
interface no longer has to use the '-interpreter-exec console ...' hack.
> Maybe nobody else thinks this, but adding an additional program into
> the Eclipse/GDB combination seems like a far more complicated
> approach, and has its own reliability issues, such as when the thin
> client crashes or hangs.
(1) I do not see that Eclipse should run another program, it is up to Eclipse
which way it would be implemented there. IIUC Eclipse would use GNU
readline classes, with no new process involved.
(2) New program (*) is better, when it means reducing the codebase of the
original program (=removing CLI from current GDB codebase; in the future).
(*) I do not say CLI-on-top-of-MI client needs to really run as a separate
UNIX process, but it should be possible to run it that way. In reality it
may be something like just linkage with libgdb.a.
> But to pull back and take a little broader view, in a system of
> software components, I think GDB has a useful role as a server that
> can have multiple frontends, telnet-like sessions, scripted drivers,
> and so forth.
I agree. Just GDB should not directly interface by CLI, this has been already
a pain and it is no longer well extensible. This also currently means that
any '--More--' prompt will stop all the other connected clients, so
a forgotten '--More--' prompt in the lab will prevent the designer to connect
to the same GDB from home anyway.
I will hopefully get to coding some very basic CLI-on-top-of-MI client to see
how practically usable it is.
Thanks,
Jan
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [patch] MI telnet service
2012-07-25 19:27 ` Jan Kratochvil
2012-07-26 0:02 ` Stan Shebs
@ 2012-08-03 11:36 ` Jan Kratochvil
1 sibling, 0 replies; 20+ messages in thread
From: Jan Kratochvil @ 2012-08-03 11:36 UTC (permalink / raw)
To: Marc Khouzam; +Cc: 'Abid, Hafiz', 'gdb-patches@sourceware.org'
Hi Marc,
On Wed, 25 Jul 2012 21:26:53 +0200, Jan Kratochvil wrote:
> Do you find the proposal above worth investigation?
It is still very poor but for very very basic use it works, with readline.
Only as single CLI over single MI, no multiplexing yet etc.:
http://git.jankratochvil.net/?p=gdbmicli.git;a=tree
git clone git://git.jankratochvil.net/gdbmicli;cd gdbmicli;make;./gdbmicli
I get to implement more, like completion, terminal handling (CTRL-C) etc.
I find it will simplify a lot of things like always forgotten
target_terminal_ours calls etc.
Thanks,
Jan
(gdb) p 1
$1 = 1
(gdb) file true
Reading symbols from /bin/true...Reading symbols from /usr/lib/debug/bin/true.debug...done.
done.
(gdb) b main
Breakpoint 1 at 0x401050: file true.c, line 59.
(gdb) run
Starting program: /bin/true
Breakpoint 1, main (argc=1, argv=0x7fffffffdab8) at true.c:59
59 if (argc == 2)
(gdb) q
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [patch] MI telnet service
2012-07-25 13:02 ` Jan Kratochvil
2012-07-25 14:12 ` Marc Khouzam
@ 2012-07-25 14:16 ` Abid, Hafiz
2012-07-25 19:29 ` Jan Kratochvil
1 sibling, 1 reply; 20+ messages in thread
From: Abid, Hafiz @ 2012-07-25 14:16 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
Hi Jan,
Thanks for the review. Some comments inline.
> -----Original Message-----
> From: Jan Kratochvil [mailto:jan.kratochvil@redhat.com]
> Sent: Wednesday, July 25, 2012 2:02 PM
> To: Abid, Hafiz
> Cc: gdb-patches@sourceware.org
> Subject: Re: [patch] MI telnet service
>
> Hi Abid,
>
> the testcase is mixing host vs. target execution. You connect to the
> port
> from the .c program but that can run on a different machine than the
> .exp
> program is running. It would be better to connect to the port from the
> .exp
> file, in fact you do not need any .c program there at all. Or if the
> test
> would be done from .c file then you need to skip the test if [is_remote
> target].
I agree that it is better to just test the port from exp file and drop the c file.
> (It could run if the target is remote but still "localhost" or "stdio"
> but
> that is currently not supported in the GDB testsuite.)
>
> "gdbmitel" is too cryptic, the option can be very long, with words
> separated
> by dashes.
Ok.
>
> Have you talked to Eclipse maintainers why they do not provide separate
> console? In fact they do, I find it more fits there. If communication
> with
> Eclipse developers has failed GDB can implement it, but I find it more
> as
> Eclipse workaround.
>
> It does not support IPv6. GDB currently also does not (for connections
> to
> gdbserver) but that is a bug, I tried to fix it, I hope to resurrect
> the patch
> sometimes again:
> [patch] IPv6 support for gdbserver
> http://sourceware.org/ml/gdb-patches/2006-09/msg00192.html
> http://sourceware.org/ml/gdb-patches/2006-10/msg00073.html
>
> Opening the port INADDR_ANY should be possible but only with some
> additional
> -start-telnet-service option, services usually open only with
> INADDR_LOOPBACK
> by default. Still there is missing security on multi-user systems, this
> is one
> of the reasons an interface through Eclipse would be more logical to
> me.
>
> You do not print any errors, such as in:
> +/* This is wrapper over recv. If recv returns some error then it
> closes
> + the socket. */
> +static int
> +receive (int *fd, void *buf, size_t n, int flags)
> I believe it could be output as:
> Node: GDB/MI Stream Records
> `"&" STRING-OUTPUT'
> The log stream contains debugging messages being produced by GDB's
> internals.
I will fix it.
>
> I find it only as a last resort possibility now without readline
> support.
> Could it support $TERM negotiation like telnetd supports and attach
> readline
> to it?
Actually I have worked on negotiating the mode with telnet and then connecting the incoming data to readline. But all this was making the patch quite complicated. So I thought to go with a simple patch first and then add the readline letter.
There has been some suggestions about making it a CLI command. But in that case, we will not be able to attach readline to the socket input as readline is already connected with CLI. I think readline cannot handle 2 parallel streams.
>
>
> Thanks,
> Jan
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [patch] MI telnet service
2012-07-25 14:16 ` Abid, Hafiz
@ 2012-07-25 19:29 ` Jan Kratochvil
0 siblings, 0 replies; 20+ messages in thread
From: Jan Kratochvil @ 2012-07-25 19:29 UTC (permalink / raw)
To: Abid, Hafiz; +Cc: gdb-patches
Hi Abid,
On Wed, 25 Jul 2012 16:16:14 +0200, Abid, Hafiz wrote:
> There has been some suggestions about making it a CLI command.
FYI I do not understand this sentence.
> But in that case, we will not be able to attach readline to the socket input
> as readline is already connected with CLI. I think readline cannot handle
> 2 parallel streams.
I see that readline has its internal state... That CLI-on-top-of-MI proposal
I sent along would solve it, each client would have its own readline.
THanks,
Jan
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2012-08-03 11:36 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-12 11:21 [patch] MI telnet service Abid, Hafiz
2012-07-12 13:19 ` Gary Benson
2012-07-12 14:37 ` Abid, Hafiz
2012-07-12 15:08 ` Gary Benson
2012-07-15 8:13 ` Jan Kratochvil
2012-07-12 13:29 ` Eli Zaretskii
2012-07-12 14:45 ` Abid, Hafiz
2012-07-14 19:37 ` Marc Khouzam
2012-07-24 16:17 ` Abid, Hafiz
2012-07-24 19:06 ` Marc Khouzam
2012-07-24 20:15 ` Marc Khouzam
2012-07-17 17:53 ` Vladimir Prus
2012-07-25 13:02 ` Jan Kratochvil
2012-07-25 14:12 ` Marc Khouzam
2012-07-25 19:27 ` Jan Kratochvil
2012-07-26 0:02 ` Stan Shebs
2012-07-26 11:51 ` Jan Kratochvil
2012-08-03 11:36 ` Jan Kratochvil
2012-07-25 14:16 ` Abid, Hafiz
2012-07-25 19:29 ` Jan Kratochvil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox