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


Thanks.

On Thursday 14 January 2010 09:27:21, Vladimir Prus wrote:
> On Wednesday 13 January 2010 18:49:15 Pedro Alves wrote:
> 
> > > -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);
> > > -}
> > 
> > If you're removing this, please delete the declaration in
> > inferior.h as well.
> > 
> > 
> > A generic comment: it is good (and the common practice) to name
> > the FOO command callbacks as "FOO_command" or
> > "set_FOO_command"/"show_FOO_command", so that we can find a command
> > implementation easily by just guessing and grepping.
> 
> I am not sure I follow you. tty_command, although named as FOO_command,
> was not actually used as command anywhere I can see. 

Sure is:

 (gdb) tty
 Argument required (filename to set it to.).

 (gdb) help tty
 Set terminal for future runs of program being debugged.
 Usage: set inferior-tty /dev/pts/1

See:

-Usage: set inferior-tty /dev/pts/1"), NULL, NULL, &setlist, &showlist);
+Usage: set inferior-tty /dev/pts/1"), 
+                           notice_inferior_io_terminal_set, 
+                           show_inferior_io_terminal, 
+                           &setlist, &showlist);
   add_com_alias ("tty", "set inferior-tty", class_alias, 0);
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

> Or do you refer to some other function?

-Usage: set inferior-tty /dev/pts/1"), NULL, NULL, &setlist, &showlist);
+Usage: set inferior-tty /dev/pts/1"), 
+                           notice_inferior_io_terminal_set, 
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+                           show_inferior_io_terminal, 
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+                           &setlist, &showlist);
   add_com_alias ("tty", "set inferior-tty", class_alias, 0);

Better choices would be: set_inferior_tty_command/show_inferior_tty_command
or set_inferior_tty/show_inferior_tty.

On Thursday 14 January 2010 09:27:21, Vladimir Prus wrote:
>    Per-inferior args and tty.

and environ.

>     
>         * 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().
>         (notice_inferior_io_terminal_set, show_inferior_io_terminal): New.
>         (get_inferior_args, set_inferior_args): Store inside
>         current_inferior().
>         (set_inferior_args_vector, notice_args_set): Likewise.
>         (tty_command): Remove.
>         (run_command_1): Don't free old args, as they are feed by
>         set_inferior_arg now.

Typo, "feed".

>         (run_no_args_command): Likewise.
>         (inferior_environ): Remove.
>         (run_command_1): Use environemnt of the current inferior.

Typo, "environemnt".

>         (environtment_info, set_environment_command)

Typo, "environtment_info".

>         (unset_environment_command, path_info, path_command): Likewise.
>         (_initialize_infcmd): Adjust variables.  Do no init inferior_environ.

Typo, "Do no".

>         * 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 environemnt of the current
>         inferior.

Typo, "environemnt".

>         (_initialize_mi_cmd_env): Adjust for disappearance of global
>         inferior_environ.
>         * solib.c (solib_find): Use environment of the current inferior.
> 

> +static void
> +notice_inferior_io_terminal_set (char *args, int from_tty, struct cmd_list_element *c)

Line too long.

> +{
> +  /* 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_io_terminal (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"),

Line too long.

> +                   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 ("");

Nit. there's a mismatch between this here and set_inferior_args:

> +  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 = xstrdup (newargs ? newargs : "");

This never leaves NULL args.  I guess it could.

> +  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;
>  }
>  


> @@ -87,6 +88,10 @@ free_inferior (struct inferior *inf)
>  {
>    discard_all_inferior_continuations (inf);
>    inferior_free_data (inf);
> +  xfree (inf->args);

> +  xfree (inf->argv);

Hmm, I was going to say that this usually leaks argv[0..argc], and
you should use freeargv, but, I now see that the only caller
of set_inferior_args_vector is captured_main, and the
argc,argv are presently a shallow copy of `main's arguments, so
definitely not ok to free this as is.  This definitely needs a
comment in the description of inferior->argv, expanded from
the comment that was in inferior_argv.

> +  xfree (inf->terminal);
> +  free_environ (inf->environment);
>    xfree (inf->private);
>    xfree (inf);
>  }

-- 
Pedro Alves


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