Re: [patch] [gdbserver] Fix multi-GB error log files [doc review]

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Wed, 16 Mar 2011 11:13:41 +0100, Pedro Alves wrote:
> Yes, I'm willing to give that a try.  I do think your patch solves
> it in a way that makes sense: without --once, the default, we leave the
> socket open; with --once, we release it as soon as possible, so the user
> can still open several debug sessions with the same port, if he's okay
> with not being able to reconnect back to the same gdbserver.

Considering this as an approval after 7.3 has been branched.

I will check it in after the new gdb/doc/ part approval.


Thanks,
Jan


gdb/gdbserver/
2011-04-17  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* remote-utils.c (handle_accept_event): Close LISTEN_DESC only if
	RUN_ONCE.  Comment for the LISTEN_DESC delete_file_handler call.
	(remote_prepare): New function with most of the TCP code from ...
	(remote_open): ... here.  Detect PORT here unconditionally.  Move also
	setting transport_is_reliable.
	* server.c (run_once): New variable.
	(gdbserver_usage): Document it.
	(main): Set run_once for `--once'.  Call remote_prepare.  Exit after
	the first run if RUN_ONCE.
	* server.h (run_once, remote_prepare): New declarations.

gdb/doc/
2011-04-17  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.texinfo (Multi-Process Mode for @code{gdbserver}): Mention --once.
	(Other Command-Line Arguments for @code{gdbserver}): New paragraph for
	--once.

gdb/testsuite/
2011-04-17  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/solib-disc.exp: Set gdbserver_reconnect_p.
	* lib/gdb.exp (gdb_init): Clear gdbserver_reconnect_p.
	* lib/gdbserver-support.exp (gdbserver_start): Add `--once' if
	!gdbserver_reconnect_p..
	(gdbserver_reconnect): Call error if !gdbserver_reconnect_p..

--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -16228,9 +16228,9 @@ or process ID to attach, use the @option{--multi} command line option.
 Then you can connect using @kbd{target extended-remote} and start
 the program you want to debug.
 
-@code{gdbserver} does not automatically exit in multi-process mode.
-You can terminate it by using @code{monitor exit}
-(@pxref{Monitor Commands for gdbserver}).
+In multi-process mode @code{gdbserver} does not automatically exit unless you
+use the option @option{--once}.  You can terminate it by using
+@code{monitor exit} (@pxref{Monitor Commands for gdbserver}).
 
 @subsubsection Other Command-Line Arguments for @code{gdbserver}
 
@@ -16240,6 +16240,13 @@ status information about the debugging process.  The
 remote protocol debug output.  These options are intended for
 @code{gdbserver} development and for bug reports to the developers.
 
+The @option{--once} will terminate @code{gdbserver} after the first connection
+with remote @value{GDBN} has closed.  By default @code{gdbserver} keeps the
+listening TCP port open so that is is always available in the case a reconnect
+is needed.  Using the @option{--once} will close the port upon the first
+connection of @value{GDBN} and such @code{gdbserver} then does not support
+reconnect from new @value{GDBN}.
+
 The @option{--wrapper} option specifies a wrapper to launch programs
 for debugging.  The option should be followed by the name of the
 wrapper, then any command-line arguments to pass to the wrapper, then
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -170,14 +170,21 @@ handle_accept_event (int err, gdb_client_data client_data)
 	      (char *) &tmp, sizeof (tmp));
 
 #ifndef USE_WIN32API
-  close (listen_desc);		/* No longer need this */
-
   signal (SIGPIPE, SIG_IGN);	/* If we don't do this, then gdbserver simply
 				   exits when the remote side dies.  */
+#endif
+
+  if (run_once)
+    {
+#ifndef USE_WIN32API
+      close (listen_desc);		/* No longer need this */
 #else
-  closesocket (listen_desc);	/* No longer need this */
+      closesocket (listen_desc);	/* No longer need this */
 #endif
+    }
 
+  /* Even if !RUN_ONCE no longer notice new connections.  Still keep the
+     descriptor open for add_file_handler to wait for a new connection.  */
   delete_file_handler (listen_desc);
 
   /* Convert IP address to string.  */
@@ -200,6 +207,62 @@ handle_accept_event (int err, gdb_client_data client_data)
   return 0;
 }
 
+/* Prepare for a later connection to a remote debugger.
+   NAME is the filename used for communication.  */
+
+void
+remote_prepare (char *name)
+{
+  char *port_str;
+#ifdef USE_WIN32API
+  static int winsock_initialized;
+#endif
+  int port;
+  struct sockaddr_in sockaddr;
+  socklen_t tmp;
+  char *port_end;
+
+  port_str = strchr (name, ':');
+  if (port_str == NULL)
+    {
+      transport_is_reliable = 0;
+      return;
+    }
+
+  port = strtoul (port_str + 1, &port_end, 10);
+  if (port_str[1] == '\0' || *port_end != '\0')
+    fatal ("Bad port argument: %s", name);
+
+#ifdef USE_WIN32API
+  if (!winsock_initialized)
+    {
+      WSADATA wsad;
+
+      WSAStartup (MAKEWORD (1, 0), &wsad);
+      winsock_initialized = 1;
+    }
+#endif
+
+  listen_desc = socket (PF_INET, SOCK_STREAM, IPPROTO_TCP);
+  if (listen_desc == -1)
+    perror_with_name ("Can't open socket");
+
+  /* Allow rapid reuse of this port. */
+  tmp = 1;
+  setsockopt (listen_desc, SOL_SOCKET, SO_REUSEADDR, (char *) &tmp,
+	      sizeof (tmp));
+
+  sockaddr.sin_family = PF_INET;
+  sockaddr.sin_port = htons (port);
+  sockaddr.sin_addr.s_addr = INADDR_ANY;
+
+  if (bind (listen_desc, (struct sockaddr *) &sockaddr, sizeof (sockaddr))
+      || listen (listen_desc, 1))
+    perror_with_name ("Can't bind address");
+
+  transport_is_reliable = 1;
+}
+
 /* Open a connection to a remote debugger.
    NAME is the filename used for communication.  */
 
@@ -274,8 +337,6 @@ remote_open (char *name)
 
       fprintf (stderr, "Remote debugging using %s\n", name);
 
-      transport_is_reliable = 0;
-
       enable_async_notification (remote_desc);
 
       /* Register the event loop handler.  */
@@ -284,64 +345,22 @@ remote_open (char *name)
     }
   else
     {
-#ifdef USE_WIN32API
-      static int winsock_initialized;
-#endif
       int port;
+      socklen_t len;
       struct sockaddr_in sockaddr;
-      socklen_t tmp;
-      char *port_end;
 
-      port = strtoul (port_str + 1, &port_end, 10);
-      if (port_str[1] == '\0' || *port_end != '\0')
-	fatal ("Bad port argument: %s", name);
-
-#ifdef USE_WIN32API
-      if (!winsock_initialized)
-	{
-	  WSADATA wsad;
-
-	  WSAStartup (MAKEWORD (1, 0), &wsad);
-	  winsock_initialized = 1;
-	}
-#endif
-
-      listen_desc = socket (PF_INET, SOCK_STREAM, IPPROTO_TCP);
-      if (listen_desc == -1)
-	perror_with_name ("Can't open socket");
-
-      /* Allow rapid reuse of this port. */
-      tmp = 1;
-      setsockopt (listen_desc, SOL_SOCKET, SO_REUSEADDR, (char *) &tmp,
-		  sizeof (tmp));
-
-      sockaddr.sin_family = PF_INET;
-      sockaddr.sin_port = htons (port);
-      sockaddr.sin_addr.s_addr = INADDR_ANY;
-
-      if (bind (listen_desc, (struct sockaddr *) &sockaddr, sizeof (sockaddr))
-	  || listen (listen_desc, 1))
-	perror_with_name ("Can't bind address");
-
-      /* If port is zero, a random port will be selected, and the
-	 fprintf below needs to know what port was selected.  */
-      if (port == 0)
-	{
-	  socklen_t len = sizeof (sockaddr);
-	  if (getsockname (listen_desc,
-			   (struct sockaddr *) &sockaddr, &len) < 0
-	      || len < sizeof (sockaddr))
-	    perror_with_name ("Can't determine port");
-	  port = ntohs (sockaddr.sin_port);
-	}
+      len = sizeof (sockaddr);
+      if (getsockname (listen_desc,
+		       (struct sockaddr *) &sockaddr, &len) < 0
+	  || len < sizeof (sockaddr))
+	perror_with_name ("Can't determine port");
+      port = ntohs (sockaddr.sin_port);
 
       fprintf (stderr, "Listening on port %d\n", port);
       fflush (stderr);
 
       /* Register the event loop handler.  */
       add_file_handler (listen_desc, handle_accept_event, NULL);
-
-      transport_is_reliable = 1;
     }
 }
 
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -40,6 +40,9 @@ static int extended_protocol;
 static int response_needed;
 static int exit_requested;
 
+/* --once: Exit after the first connection has closed.  */
+int run_once;
+
 int multi_process;
 int non_stop;
 
@@ -2312,7 +2315,9 @@ gdbserver_usage (FILE *stream)
 	   "  --debug               Enable general debugging output.\n"
 	   "  --remote-debug        Enable remote protocol debugging output.\n"
 	   "  --version             Display version information and exit.\n"
-	   "  --wrapper WRAPPER --  Run WRAPPER to start new programs.\n");
+	   "  --wrapper WRAPPER --  Run WRAPPER to start new programs.\n"
+	   "  --once                Exit after the first connection has "
+								  "closed.\n");
   if (REPORT_BUGS_TO[0] && stream == stdout)
     fprintf (stream, "Report bugs to \"%s\".\n", REPORT_BUGS_TO);
 }
@@ -2536,6 +2541,8 @@ main (int argc, char *argv[])
 		}
 	    }
 	}
+      else if (strcmp (*next_arg, "--once") == 0)
+	run_once = 1;
       else
 	{
 	  fprintf (stderr, "Unknown argument: %s\n", *next_arg);
@@ -2648,6 +2655,8 @@ main (int argc, char *argv[])
       exit (1);
     }
 
+  remote_prepare (port);
+
   while (1)
     {
       noack_mode = 0;
@@ -2676,7 +2685,7 @@ main (int argc, char *argv[])
 	 getpkt to fail; close the connection and reopen it at the
 	 top of the loop.  */
 
-      if (exit_requested)
+      if (exit_requested || run_once)
 	{
 	  detach_or_kill_for_exit ();
 	  exit (0);
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -355,6 +355,7 @@ extern int disable_packet_Tthread;
 extern int disable_packet_qC;
 extern int disable_packet_qfThreadInfo;
 
+extern int run_once;
 extern int multi_process;
 extern int non_stop;
 
@@ -406,6 +407,7 @@ int putpkt (char *buf);
 int putpkt_binary (char *buf, int len);
 int putpkt_notif (char *buf);
 int getpkt (char *buf);
+void remote_prepare (char *name);
 void remote_open (char *name);
 void remote_close (void);
 void write_ok (char *buf);
--- a/gdb/testsuite/gdb.base/solib-disc.exp
+++ b/gdb/testsuite/gdb.base/solib-disc.exp
@@ -19,6 +19,7 @@ if {[skip_shlib_tests]} {
     return 0
 }
 
+set gdbserver_reconnect_p 1
 if { [info proc gdb_reconnect] == "" } {
     return 0
 }
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2809,6 +2809,11 @@ proc gdb_init { args } {
     # especially having color output turned on can cause tests to fail.
     setenv GREP_OPTIONS ""
 
+    # Clear $gdbserver_reconnect_p.
+    global gdbserver_reconnect_p
+    set gdbserver_reconnect_p 1
+    unset gdbserver_reconnect_p
+
     return [eval default_gdb_init $args];
 }
 
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -218,10 +218,21 @@ proc gdbserver_start { options arguments } {
 
 	# Fire off the debug agent.
 	set gdbserver_command "$gdbserver"
+
+	# If gdbserver_reconnect will be called $gdbserver_reconnect_p must be
+	# set to true already during gdbserver_start.
+	global gdbserver_reconnect_p
+	if {![info exists gdbserver_reconnect_p] || !$gdbserver_reconnect_p} {
+	    # GDB client could accidentally connect to a stale server.
+	    append gdbserver_command " --once"
+	}
+
 	if { $options != "" } {
 	    append gdbserver_command " $options"
 	}
+
 	append gdbserver_command " :$portnum"
+
 	if { $arguments != "" } {
 	    append gdbserver_command " $arguments"
 	}
@@ -315,6 +326,12 @@ proc gdbserver_reconnect { } {
     global gdbserver_protocol
     global gdbserver_gdbport
 
+    global gdbserver_reconnect_p;
+    if {![info exists gdbserver_reconnect_p] || !$gdbserver_reconnect_p} {
+	error "gdbserver_reconnect_p is not set before gdbserver_reconnect"
+	return 0
+    }
+
     return [gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport]
 }