This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Per-inferior program arguments and io terminal


On Thursday 14 January 2010 18:49:10 Pedro Alves wrote:

> > @@ -87,6 +88,10 @@ free_inferior (struct inferior *inf)
> >  {
> >    discard_all_inferior_continuations (inf);
> >    inferior_free_data (inf);
> > +  xfree (inf->args);
> > +  xfree (inf->argv);
> > +  xfree (inf->terminal);
> > +  free_environ (inf->environment);
> >    xfree (inf->private);
> 
> Maybe I confused you with saying "shallow" copy.  This
> argv comes from a direct `argv' pointer copy:

...

> This is only called from captured_main, like so:
> 
>  set_inferior_args_vector (argc - optind, &argv[optind]);
 
...

> so, freeing that would be equivalent to:
> 
>  int
>  main (int argc, char **argv)
>  {
>     free (argv);
>  }
> 
> which is not kosher (try it).   You should _not_ free inferior->argv.

Ouch. Here's yet another revision.

> >qr
> Warning: trailing whitespace in lines 230,238 of gdb/solib.c
> Warning: trailing whitespace in lines 664,673,683 of gdb/main.c
> Warning: trailing whitespace in line 266 of gdb/mi/mi-cmd-env.c
> Warning: trailing whitespace in lines 129,186,200,213,2653,2654,2655 of gdb/infcmd.c
> Refreshed patch per-inferior-args3.diff

Also fixed now.

- Volodya
commit 5ea1aab67573f59d22bedb6841d1fabdabed261c
Author: Vladimir Prus <vladimir@codesourcery.com>
Date:   Mon Dec 21 15:28:03 2009 +0300

    Per-inferior args and tty and environment.
    
    	* infcmd.c (inferior_args): Rename to ...
    	(inferior_args_scratch): ... this.
    	(inferior_io_terminal): Rename to ...
    	(inferior_io_terminal_scratch): ... this.
    	(inferior_argc, inferior_argv): Remove.
    	(set_inferior_io_terminal, get_inferior_io_terminal): Store
    	inside current_inferior().
    	(set_inferior_tty_command, show_inferior_tty_command): New.
    	(get_inferior_args, set_inferior_args): Store inside
    	current_inferior().
    	(notice_args_set): Likewise and rename to...
    	(set_args_command): ... this.
    	(set_inferior_args_vector): Likewise.
    	(notice_args_read): Rename to...
    	(show_args_command): ...new.
    	(tty_command): Remove.
    	(run_command_1): Don't free old args, as they are freed by
    	set_inferior_arg now.
    	(run_no_args_command): Likewise.
    	(inferior_environ): Remove.
    	(run_command_1): Use environemnt of the current inferior.
    	(environment_info, set_environment_command)
    	(unset_environment_command, path_info, path_command): Likewise.
    	(_initialize_infcmd): Adjust for function and variable renames.
    	Do not init inferior_environ.
    	* inferior.h (set_inferior_arg): Adjust prototype.
    	(struct inferior): New fields args, argc, argv, terminal, environment.
    	(inferior_environ): Remove declaration.
    	* inferior.c (free_inferior): Free new fields.
    	(add_inferior_silent): Initialize 'environment' field.
    	* main.c (captured_main): Set arguments only after the initial
    	inferior has been created.  Set set_inferior_io_terminal,
    	not tty_command.
    	* mi/mi-main.c (mi_cmd_env_path): Use environment of the current
    	inferior.
    	(_initialize_mi_cmd_env): Adjust for disappearance of global
    	inferior_environ.
    	* solib.c (solib_find): Use environment of the current inferior.

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 21a2233..f99a4ae 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -126,20 +126,17 @@ void _initialize_infcmd (void);
 #define ERROR_NO_INFERIOR \
    if (!target_has_execution) error (_("The program is not being run."));
 
-/* String containing arguments to give to the program, separated by spaces.
-   Empty string (pointer to '\0') means no args.  */
+/* Scratch area where string containing arguments to give to the program will be
+   stored by 'set args'.  As soon as anything is stored, notice_args_set will
+   move it into per-inferior storage.  Arguments are separated by spaces. Empty
+   string (pointer to '\0') means no args.  */
 
-static char *inferior_args;
+static char *inferior_args_scratch;
 
-/* The inferior arguments as a vector.  If INFERIOR_ARGC is nonzero,
-   then we must compute INFERIOR_ARGS from this (via the target).  */
+/* Scratch area where 'set inferior-tty' will store user-provided value.
+   We'll immediate copy it into per-inferior storage.  */
 
-static int inferior_argc;
-static char **inferior_argv;
-
-/* File name for default use for standard in/out in the inferior.  */
-
-static char *inferior_io_terminal;
+static char *inferior_io_terminal_scratch;
 
 /* Pid of our debugged inferior, or 0 if no inferior now.
    Since various parts of infrun.c test this to see whether there is a program
@@ -166,80 +163,97 @@ int stop_stack_dummy;
 
 int stopped_by_random_signal;
 
-/* Environment to use for running inferior,
-   in format described in environ.h.  */
-
-struct gdb_environ *inferior_environ;
 
 /* Accessor routines. */
 
+/* Set the io terminal for the current inferior.  Ownership of
+   TERMINAL_NAME is not transferred.  */
+
 void 
 set_inferior_io_terminal (const char *terminal_name)
 {
-  if (inferior_io_terminal)
-    xfree (inferior_io_terminal);
-
-  if (!terminal_name)
-    inferior_io_terminal = NULL;
-  else
-    inferior_io_terminal = xstrdup (terminal_name);
+  xfree (current_inferior ()->terminal);
+  current_inferior ()->terminal = terminal_name ? xstrdup (terminal_name) : 0;
 }
 
 const char *
 get_inferior_io_terminal (void)
 {
-  return inferior_io_terminal;
+  return current_inferior ()->terminal;
+}
+
+static void
+set_inferior_tty_command (char *args, int from_tty,
+			  struct cmd_list_element *c)
+{
+  /* CLI has assigned the user-provided value to inferior_io_terminal_scratch.
+     Now route it to current inferior.  */
+  set_inferior_io_terminal (inferior_io_terminal_scratch);
+}
+
+static void
+show_inferior_tty_command (struct ui_file *file, int from_tty,
+			   struct cmd_list_element *c, const char *value)
+{
+  /* Note that we ignore the passed-in value in favor of computing it
+     directly.  */
+  fprintf_filtered (gdb_stdout,
+		    _("argument list to give program being debugged when "
+		      "it is started is %s"),
+		    get_inferior_io_terminal ());
 }
 
 char *
 get_inferior_args (void)
 {
-  if (inferior_argc != 0)
+  if (current_inferior ()->argc != 0)
     {
-      char *n, *old;
+      char *n;
 
-      n = construct_inferior_arguments (inferior_argc, inferior_argv);
-      old = set_inferior_args (n);
-      xfree (old);
+      n = construct_inferior_arguments (current_inferior ()->argc,
+					current_inferior ()->argv);
+      set_inferior_args (n);
+      xfree (n);
     }
 
-  if (inferior_args == NULL)
-    inferior_args = xstrdup ("");
+  if (current_inferior ()->args == NULL)
+    current_inferior ()->args = xstrdup ("");
 
-  return inferior_args;
+  return current_inferior ()->args;
 }
 
-char *
+/* Set the arguments for the current inferior.  Ownership of
+   NEWARGS is not transferred.  */
+
+void
 set_inferior_args (char *newargs)
 {
-  char *saved_args = inferior_args;
-
-  inferior_args = newargs;
-  inferior_argc = 0;
-  inferior_argv = 0;
-
-  return saved_args;
+  xfree (current_inferior ()->args);
+  current_inferior ()->args = newargs ? xstrdup (newargs) : NULL;
+  current_inferior ()->argc = 0;
+  current_inferior ()->argv = 0;
 }
 
 void
 set_inferior_args_vector (int argc, char **argv)
 {
-  inferior_argc = argc;
-  inferior_argv = argv;
+  current_inferior ()->argc = argc;
+  current_inferior ()->argv = argv;
 }
 
 /* Notice when `set args' is run.  */
 static void
-notice_args_set (char *args, int from_tty, struct cmd_list_element *c)
+set_args_command (char *args, int from_tty, struct cmd_list_element *c)
 {
-  inferior_argc = 0;
-  inferior_argv = 0;
+  /* CLI has assigned the user-provided value to inferior_args_scratch.
+     Now route it to current inferior.  */
+  set_inferior_args (inferior_args_scratch);
 }
 
 /* Notice when `show args' is run.  */
 static void
-notice_args_read (struct ui_file *file, int from_tty,
-		  struct cmd_list_element *c, const char *value)
+show_args_command (struct ui_file *file, int from_tty,
+		   struct cmd_list_element *c, const char *value)
 {
   /* Note that we ignore the passed-in value in favor of computing it
      directly.  */
@@ -369,15 +383,6 @@ strip_bg_char (char **args)
   return 0;
 }
 
-void
-tty_command (char *file, int from_tty)
-{
-  if (file == 0)
-    error_no_arg (_("terminal name for running target process"));
-
-  set_inferior_io_terminal (file);
-}
-
 /* Common actions to take after creating any sort of inferior, by any
    means (running, attaching, connecting, et cetera).  The target
    should be stopped.  */
@@ -536,10 +541,7 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main)
 
       /* If there were other args, beside '&', process them. */
       if (args)
-	{
-          char *old_args = set_inferior_args (xstrdup (args));
-          xfree (old_args);
-	}
+	set_inferior_args (args);
     }
 
   if (from_tty)
@@ -559,7 +561,7 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main)
   /* We call get_inferior_args() because we might need to compute
      the value now.  */
   target_create_inferior (exec_file, get_inferior_args (),
-			  environ_vector (inferior_environ), from_tty);
+			  environ_vector (current_inferior ()->environment), from_tty);
 
   /* We're starting off a new process.  When we get out of here, in
      non-stop mode, finish the state of all threads of that process,
@@ -594,8 +596,7 @@ run_command (char *args, int from_tty)
 static void
 run_no_args_command (char *args, int from_tty)
 {
-  char *old_args = set_inferior_args (xstrdup (""));
-  xfree (old_args);
+  set_inferior_args ("");
 }
 
 
@@ -1699,7 +1700,7 @@ environment_info (char *var, int from_tty)
 {
   if (var)
     {
-      char *val = get_in_environ (inferior_environ, var);
+      char *val = get_in_environ (current_inferior ()->environment, var);
       if (val)
 	{
 	  puts_filtered (var);
@@ -1716,7 +1717,7 @@ environment_info (char *var, int from_tty)
     }
   else
     {
-      char **vector = environ_vector (inferior_environ);
+      char **vector = environ_vector (current_inferior ()->environment);
       while (*vector)
 	{
 	  puts_filtered (*vector++);
@@ -1781,10 +1782,10 @@ set_environment_command (char *arg, int from_tty)
       printf_filtered (_("\
 Setting environment variable \"%s\" to null value.\n"),
 		       var);
-      set_in_environ (inferior_environ, var, "");
+      set_in_environ (current_inferior ()->environment, var, "");
     }
   else
-    set_in_environ (inferior_environ, var, val);
+    set_in_environ (current_inferior ()->environment, var, val);
   xfree (var);
 }
 
@@ -1797,12 +1798,12 @@ unset_environment_command (char *var, int from_tty)
          Ask for confirmation if reading from the terminal.  */
       if (!from_tty || query (_("Delete all environment variables? ")))
 	{
-	  free_environ (inferior_environ);
-	  inferior_environ = make_environ ();
+	  free_environ (current_inferior ()->environment);
+	  current_inferior ()->environment = make_environ ();
 	}
     }
   else
-    unset_in_environ (inferior_environ, var);
+    unset_in_environ (current_inferior ()->environment, var);
 }
 
 /* Handle the execution path (PATH variable) */
@@ -1813,7 +1814,7 @@ static void
 path_info (char *args, int from_tty)
 {
   puts_filtered ("Executable and object file path: ");
-  puts_filtered (get_in_environ (inferior_environ, path_var_name));
+  puts_filtered (get_in_environ (current_inferior ()->environment, path_var_name));
   puts_filtered ("\n");
 }
 
@@ -1825,13 +1826,13 @@ path_command (char *dirname, int from_tty)
   char *exec_path;
   char *env;
   dont_repeat ();
-  env = get_in_environ (inferior_environ, path_var_name);
+  env = get_in_environ (current_inferior ()->environment, path_var_name);
   /* Can be null if path is not set */
   if (!env)
     env = "";
   exec_path = xstrdup (env);
   mod_path (dirname, &exec_path);
-  set_in_environ (inferior_environ, path_var_name, exec_path);
+  set_in_environ (current_inferior ()->environment, path_var_name, exec_path);
   xfree (exec_path);
   if (from_tty)
     path_info ((char *) NULL, from_tty);
@@ -2646,19 +2647,22 @@ _initialize_infcmd (void)
 
   /* add the filename of the terminal connected to inferior I/O */
   add_setshow_filename_cmd ("inferior-tty", class_run,
-			    &inferior_io_terminal, _("\
+			    &inferior_io_terminal_scratch, _("\
 Set terminal for future runs of program being debugged."), _("\
 Show terminal for future runs of program being debugged."), _("\
-Usage: set inferior-tty /dev/pts/1"), NULL, NULL, &setlist, &showlist);
+Usage: set inferior-tty /dev/pts/1"),
+			    set_inferior_tty_command,
+			    show_inferior_tty_command,
+			    &setlist, &showlist);
   add_com_alias ("tty", "set inferior-tty", class_alias, 0);
 
   add_setshow_optional_filename_cmd ("args", class_run,
-				     &inferior_args, _("\
+				     &inferior_args_scratch, _("\
 Set argument list to give program being debugged when it is started."), _("\
 Show argument list to give program being debugged when it is started."), _("\
 Follow this command with any number of args, to be passed to the program."),
-				     notice_args_set,
-				     notice_args_read,
+				     set_args_command,
+				     show_args_command,
 				     &setlist, &showlist);
 
   c = add_cmd ("environment", no_class, environment_info, _("\
@@ -2855,7 +2859,4 @@ Register name as argument means describe only that register."));
 
   add_info ("vector", vector_info,
 	    _("Print the status of the vector unit\n"));
-
-  inferior_environ = make_environ ();
-  init_environ (inferior_environ);
 }
diff --git a/gdb/inferior.c b/gdb/inferior.c
index d27a3e3..0667bfa 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -29,6 +29,7 @@
 #include "gdbthread.h"
 #include "gdbcore.h"
 #include "symfile.h"
+#include "environ.h"
 
 void _initialize_inferiors (void);
 
@@ -87,6 +88,9 @@ free_inferior (struct inferior *inf)
 {
   discard_all_inferior_continuations (inf);
   inferior_free_data (inf);
+  xfree (inf->args);
+  xfree (inf->terminal);
+  free_environ (inf->environment);
   xfree (inf->private);
   xfree (inf);
 }
@@ -124,6 +128,9 @@ add_inferior_silent (int pid)
   inf->next = inferior_list;
   inferior_list = inf;
 
+  inf->environment = make_environ ();
+  init_environ (inf->environment);
+
   inferior_alloc_data (inf);
 
   if (pid != 0)
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 9798ef1..e557d6c 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -131,8 +131,6 @@ extern int sync_execution;
 
 /* Inferior environment. */
 
-extern struct gdb_environ *inferior_environ;
-
 extern void clear_proceed_status (void);
 
 extern void proceed (CORE_ADDR, enum target_signal, int);
@@ -253,15 +251,13 @@ void set_step_info (struct frame_info *frame, struct symtab_and_line sal);
 
 /* From infcmd.c */
 
-extern void tty_command (char *, int);
-
 extern void post_create_inferior (struct target_ops *, int);
 
 extern void attach_command (char *, int);
 
 extern char *get_inferior_args (void);
 
-extern char *set_inferior_args (char *);
+extern void set_inferior_args (char *);
 
 extern void set_inferior_args_vector (int, char **);
 
@@ -427,6 +423,25 @@ struct inferior
   /* The program space bound to this inferior.  */
   struct program_space *pspace;
 
+  /* The arguments string to use when running.  */
+  char *args;
+
+  /* The size of elements in argv.  */
+  int argc;
+
+  /* The vector version of arguments.  If ARGC is nonzero,
+     then we must compute ARGS from this (via the target).
+     This is always coming from main's argv and therefore
+     should never be freed.  */
+  char **argv;
+
+  /* The name of terminal device to use for I/O.  */
+  char *terminal;
+
+  /* Environment to use for running inferior,
+     in format described in environ.h.  */
+  struct gdb_environ *environment;
+
   /* See the definition of stop_kind above.  */
   enum stop_kind stop_soon;
 
diff --git a/gdb/main.c b/gdb/main.c
index 0cc0f75..e261348 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -631,54 +631,6 @@ extern int gdbtk_test (char *);
 	use_windows = 0;
       }
 
-    if (set_args)
-      {
-	/* The remaining options are the command-line options for the
-	   inferior.  The first one is the sym/exec file, and the rest
-	   are arguments.  */
-	if (optind >= argc)
-	  {
-	    fprintf_unfiltered (gdb_stderr,
-				_("%s: `--args' specified but no program specified\n"),
-				argv[0]);
-	    exit (1);
-	  }
-	symarg = argv[optind];
-	execarg = argv[optind];
-	++optind;
-	set_inferior_args_vector (argc - optind, &argv[optind]);
-      }
-    else
-      {
-	/* OK, that's all the options.  */
-
-	/* The first argument, if specified, is the name of the
-	   executable.  */
-	if (optind < argc)
-	  {
-	    symarg = argv[optind];
-	    execarg = argv[optind];
-	    optind++;
-	  }
-
-	/* If the user hasn't already specified a PID or the name of a
-	   core file, then a second optional argument is allowed.  If
-	   present, this argument should be interpreted as either a
-	   PID or a core file, whichever works.  */
-	if (pidarg == NULL && corearg == NULL && optind < argc)
-	  {
-	    pid_or_core_arg = argv[optind];
-	    optind++;
-	  }
-
-	/* Any argument left on the command line is unexpected and
-	   will be ignored.  Inform the user.  */
-	if (optind < argc)
-	  fprintf_unfiltered (gdb_stderr, _("\
-Excess command line arguments ignored. (%s%s)\n"),
-			      argv[optind],
-			      (optind == argc - 1) ? "" : " ...");
-      }
     if (batch)
       quiet = 1;
   }
@@ -687,6 +639,57 @@ Excess command line arguments ignored. (%s%s)\n"),
      control of the console via the deprecated_init_ui_hook ().  */
   gdb_init (argv[0]);
 
+  /* Now that gdb_init has created the initial inferior, we're in position
+     to set args for that inferior.  */
+  if (set_args)
+    {
+      /* The remaining options are the command-line options for the
+	 inferior.  The first one is the sym/exec file, and the rest
+	 are arguments.  */
+      if (optind >= argc)
+	{
+	  fprintf_unfiltered (gdb_stderr,
+			      _("%s: `--args' specified but no program specified\n"),
+			      argv[0]);
+	  exit (1);
+	}
+      symarg = argv[optind];
+      execarg = argv[optind];
+      ++optind;
+      set_inferior_args_vector (argc - optind, &argv[optind]);
+    }
+  else
+    {
+      /* OK, that's all the options.  */
+
+      /* The first argument, if specified, is the name of the
+	 executable.  */
+      if (optind < argc)
+	{
+	  symarg = argv[optind];
+	  execarg = argv[optind];
+	  optind++;
+	}
+
+      /* If the user hasn't already specified a PID or the name of a
+	 core file, then a second optional argument is allowed.  If
+	 present, this argument should be interpreted as either a
+	 PID or a core file, whichever works.  */
+      if (pidarg == NULL && corearg == NULL && optind < argc)
+	{
+	  pid_or_core_arg = argv[optind];
+	  optind++;
+	}
+
+      /* Any argument left on the command line is unexpected and
+	 will be ignored.  Inform the user.  */
+      if (optind < argc)
+	fprintf_unfiltered (gdb_stderr, _("\
+Excess command line arguments ignored. (%s%s)\n"),
+			    argv[optind],
+			    (optind == argc - 1) ? "" : " ...");
+    }
+
   /* Lookup gdbinit files. Note that the gdbinit file name may be overriden
      during file initialization, so get_init_files should be called after
      gdb_init.  */
@@ -840,7 +843,7 @@ Can't attach to process and specify a core file at the same time."));
     }
 
   if (ttyarg != NULL)
-    catch_command_errors (tty_command, ttyarg, !batch, RETURN_MASK_ALL);
+    set_inferior_io_terminal (ttyarg);
 
   /* Error messages should no longer be distinguished with extra output. */
   error_pre_print = NULL;
diff --git a/gdb/mi/mi-cmd-env.c b/gdb/mi/mi-cmd-env.c
index d9703ee..cdd25f2 100644
--- a/gdb/mi/mi-cmd-env.c
+++ b/gdb/mi/mi-cmd-env.c
@@ -162,7 +162,7 @@ mi_cmd_env_path (char *command, char **argv, int argc)
   else
     {
       /* Otherwise, get current path to modify.  */
-      env = get_in_environ (inferior_environ, path_var_name);
+      env = get_in_environ (current_inferior ()->environment, path_var_name);
 
       /* Can be null if path is not set.  */
       if (!env)
@@ -173,9 +173,9 @@ mi_cmd_env_path (char *command, char **argv, int argc)
   for (i = argc - 1; i >= 0; --i)
     env_mod_path (argv[i], &exec_path);
 
-  set_in_environ (inferior_environ, path_var_name, exec_path);
+  set_in_environ (current_inferior ()->environment, path_var_name, exec_path);
   xfree (exec_path);
-  env = get_in_environ (inferior_environ, path_var_name);
+  env = get_in_environ (current_inferior ()->environment, path_var_name);
   ui_out_field_string (uiout, "path", env);
 }
 
@@ -260,10 +260,17 @@ mi_cmd_inferior_tty_show (char *command, char **argv, int argc)
 void 
 _initialize_mi_cmd_env (void)
 {
+  struct gdb_environ *environment;
   char *env;
 
-  /* We want original execution path to reset to, if desired later.  */
-  env = get_in_environ (inferior_environ, path_var_name);
+  /* We want original execution path to reset to, if desired later.
+     At this point, current inferior is not created, so cannot use
+     current_inferior ()->environment.  Also, there's no obvious
+     place where this code can be moved suchs that it surely run
+     before any code possibly mangles original PATH.  */
+  environment = make_environ ();
+  init_environ (environment);
+  env = get_in_environ (environment, path_var_name);
 
   /* Can be null if path is not set.  */
   if (!env)
diff --git a/gdb/solib.c b/gdb/solib.c
index 21006d8..842b27c 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -227,14 +227,16 @@ solib_find (char *in_pathname, int *fd)
 
   /* If not found, next search the inferior's $PATH environment variable. */
   if (found_file < 0 && gdb_sysroot_is_empty)
-    found_file = openp (get_in_environ (inferior_environ, "PATH"),
+    found_file = openp (get_in_environ (current_inferior ()->environment,
+					"PATH"),
 			OPF_TRY_CWD_FIRST, in_pathname, O_RDONLY | O_BINARY,
 			&temp_pathname);
 
   /* If not found, next search the inferior's $LD_LIBRARY_PATH 
      environment variable. */
   if (found_file < 0 && gdb_sysroot_is_empty)
-    found_file = openp (get_in_environ (inferior_environ, "LD_LIBRARY_PATH"),
+    found_file = openp (get_in_environ (current_inferior ()->environment,
+					"LD_LIBRARY_PATH"),
 			OPF_TRY_CWD_FIRST, in_pathname, O_RDONLY | O_BINARY,
 			&temp_pathname);
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]