This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: Proposed patch for gdb/mi 741
- From: "J. Johnston" <jjohnstn at redhat dot com>
- To: Elena Zannoni <ezannoni at redhat dot com>
- Cc: gdb-patches at sources dot redhat dot com
- Date: Fri, 08 Nov 2002 18:53:36 -0500
- Subject: Re: Proposed patch for gdb/mi 741
- Organization: Red Hat Inc.
- References: <3DA4886F.194D484D@redhat.com> <15797.30559.329315.574728@localhost.redhat.com> <3DCAE4E2.D05B3DD8@redhat.com> <15818.64935.170977.501640@localhost.redhat.com>
Elena Zannoni wrote:
>
> J. Johnston writes:
> > Elena Zannoni wrote:
> > >
> > > J. Johnston writes:
> > > > The following solves a number of problems with the mi -environment commands.
> > > > For starters, each command now has an mi cmd wrapper so arguments may use the
> > > > syntax of adding double-quotes to handle extraneous characters such as spaces.
> > > >
> > > > The -environment-pwd command now lists the output in mi syntax:
> > > >
> > > > e.g.
> > > > -environment-pwd
> > > > ^done,cwd="...."
> > > >
> > > > The -environment-directory command has been changed to output in mi format.
> > > > It also has been changed to allow for no arguments being passed.
> > > > If no arguments are passed, it behaves like the gdb dir command and resets the
> > > > source search path to the default, however, no confirmation query is performed. If an empty
> > > > string "" is passed, it is ignored so this can be used to display the current
> > > > search path without modifying it.
> > > >
> > > > e.g.
> > > > -environment-directory /usr/bin /usr/local/bin
> > > > ^done,source-path="/usr/bin:/usr/local/bin:$cdir:$cwd"
> > > >
> > > > The -environment-path command has been changed to output in mi format.
> > > > It also has been changed to allow for no arguments being passed. When
> > > > no arguments are passed, the current object search path is displayed.
> > > >
> > > > e.g.
> > > > -environment-path
> > > > ^done,path="....."
> > > >
> > > > For mi1 or below, the previous behavior is preserved by rerouting the commands
> > > > to their old cli counterparts.
> > >
> > > Hmmm, I think the testsuite changes are ok. Also the addition of the
> > > new file is fine. However I don't think we should be duplicating the
> > > mod_path() code. Would it be possible to maybe split mod_path() in 2
> > > or more functions, and have MI call one of them?
> > > I mean
> > >
> > > mod_path()
> > > {
> > > do_cli_specific_stuff;
> > > call add_path(args1);
> > > ....
> > > }
> > >
> > > mi_env_path()
> > > {
> > > do_mi_specific_stuff;
> > > call add_path(args2);
> > > .....
> > > }
> > >
> > > similarly for other code that may have been duplicated.
> > >
> > > Thanks
> > > Elena
> >
> > I have taken your advice and have added a new interface: add_path which takes
> > an additional argument. The mod_path() routine calls it one way, the mi code
> > calls it another. Of the other code I copied, there is no value add in trying
> > to make it common as I copied a few lines here and there.
> >
> > I found I had to modify the tilde_expand() prototype in defs.h to get my
> > build working. I have a combined source tree and there was a conflict with
> > tilde.h found in the readline directory. The defs.h prototype was not
> > declaring the argument as const.
> >
>
> Ah, right. readline has changed tilde_expand. You must have a newer
> version of realine installed? We cannot make that change yet, because
> we need to build with the readline in our source tree, unfortunately.
> But the upgrade of readline is coming soon. (which reminds me...)
>
Ok, maybe I am bit confused. I thought the readline directory containing the
header file is from my latest sourceware gdb checkout.
> I don't like one minor thing. The inconsistent meaning of the 'no argument'
> for env-path and env-dir. I would think it would be more intuitive if both
> would behave the same w/o argument: display the current values.
> Maybe have a "--reset" or something like that to restore the defaults?
> (suggestions welcome).
>
The proposed behavior is mimicking the gdb command line which resets
when you do a dir with no arguments (sans prompt) and shows the current
path when you do a path with no arguments. I have no problem with
adding an option to the env-dir command to make it a more explicit operation.
> Otherwise the patch is ok, except for a couple of little things. The
> comments. They need to start with a capital letter and end with a
> period, and 2 spaces after periods. (I know, it's a pain) No externs
> in .c files: include inferior.h (update Makefile) and add source_path
> to defs.h.
>
Done. I will resubmit when I add the reset option and
yes, I will remember to put the PR number in the ChangeLogs.
> >
> > gdb/ChangeLog:
> >
> > 2002-11-07 Jeff Johnston <jjohnstn@redhat.com>
> >
> > * defs.h (init_last_source_visited): New prototype.
> > (add_path): Ditto.
> > (tilde_expand): Change prototype to const char * to match readline.h.
> > * source.c (add_path): New function that adds to a specified path.
> > (mod_path): Change to call add_path.
> > (init_last_source_visited): New function to allow interfaces to
> > initialize static variable: last_source_visited.
> >
> > gdb/mi/ChangeLog:
> >
> > 2002-11-07 Jeff Johnston <jjohnstn@redhat.com>
> >
> > * mi-cmds.c (-environment-directory) Change to use mi_cmd_env_dir,
> > (-environment-cd): Change to use mi_cmd_env_cd,.
> > (-environment-pwd): Change to use mi_cmd_env_pwd.
> > (-environment-path): Change to use mi_cmd_env_path.
> > * mi-cmds.h (mi_cmd_env_cd, mi_cmd_env_dir): New prototypes.
> > (mi_cmd_env_path, mi_cmd_env_pwd): Ditto.
> > * mi-cmd-env.c: New file. Part of fix for PR gdb/741.
> > * gdbmi.texinfo (environment-cd): Update output and example.
> > (environment-pwd): Ditto.
> > (environment-dir): Update output, description, and examples.
> > (environment-path): Ditto.
> >
> > gdb/testsuite/gdb.mi/ChangeLog:
> >
> > 2002-11-07 Jeff Johnston <jjohnstn@redhat.com>
> >
> > * mi-basics.exp: Change tests for -environment-directory. Also add
> > tests for -environment-cd and -environment-pwd. Part of fix for
> > PR gdb/741.Index: defs.h
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/defs.h,v
> > retrieving revision 1.102
> > diff -u -r1.102 defs.h
> > --- defs.h 15 Oct 2002 02:16:51 -0000 1.102
> > +++ defs.h 7 Nov 2002 21:47:03 -0000
> > @@ -572,10 +572,14 @@
> >
> > extern void mod_path (char *, char **);
> >
> > +extern void add_path (char *, char **, int);
> > +
> > extern void directory_command (char *, int);
> >
> > extern void init_source_path (void);
> >
> > +extern void init_last_source_visited (void);
> > +
> > extern char *symtab_to_filename (struct symtab *);
> >
> > /* From exec.c */
> > @@ -616,7 +620,7 @@
> >
> > /* From readline (but not in any readline .h files). */
> >
> > -extern char *tilde_expand (char *);
> > +extern char *tilde_expand (const char *);
> >
> > /* Control types for commands */
> >
> > Index: source.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/source.c,v
> > retrieving revision 1.36
> > diff -u -r1.36 source.c
> > --- source.c 24 Oct 2002 21:02:53 -0000 1.36
> > +++ source.c 7 Nov 2002 21:47:03 -0000
> > @@ -357,6 +357,12 @@
> > forget_cached_source_info ();
> > }
> >
> > +void
> > +init_last_source_visited (void)
> > +{
> > + last_source_visited = NULL;
> > +}
> > +
> > /* Add zero or more directories to the front of the source path. */
> >
> > void
> > @@ -387,6 +393,18 @@
> > void
> > mod_path (char *dirname, char **which_path)
> > {
> > + add_path (dirname, which_path, 1);
> > +}
> > +
> > +/* Workhorse of mod_path. Takes an extra argument to determine
> > + if dirname should be parsed for separators that indicate multiple
> > + directories. This allows for interfaces that pre-parse the dirname
> > + and allow specification of traditional separator characters such
> > + as space or tab. */
> > +
> > +void
> > +add_path (char *dirname, char **which_path, int parse_separators)
> > +{
> > char *old = *which_path;
> > int prefix = 0;
> >
> > @@ -403,9 +421,16 @@
> > struct stat st;
> >
> > {
> > - char *separator = strchr (name, DIRNAME_SEPARATOR);
> > - char *space = strchr (name, ' ');
> > - char *tab = strchr (name, '\t');
> > + char *separator = NULL;
> > + char *space = NULL;
> > + char *tab = NULL;
> > +
> > + if (parse_separators)
> > + {
> > + separator = strchr (name, DIRNAME_SEPARATOR);
> > + space = strchr (name, ' ');
> > + tab = strchr (name, '\t');
> > + }
> >
> > if (separator == 0 && space == 0 && tab == 0)
> > p = dirname = name + strlen (name);
> > @@ -536,7 +561,8 @@
> > tinybuf[0] = DIRNAME_SEPARATOR;
> > tinybuf[1] = '\0';
> >
> > - /* If we have already tacked on a name(s) in this command, be sure they stay on the front as we tack on some more. */
> > + /* If we have already tacked on a name(s) in this command, be sure they stay
> > + on the front as we tack on some more. */
> > if (prefix)
> > {
> > char *temp, c;
> > Index: mi/mi-cmd-env.c
> > ===================================================================
> > RCS file: mi/mi-cmd-env.c
> > diff -N mi/mi-cmd-env.c
> > --- mi/mi-cmd-env.c 1 Jan 1970 00:00:00 -0000
> > +++ mi/mi-cmd-env.c 7 Nov 2002 21:47:04 -0000
> > @@ -0,0 +1,174 @@
> > +/* MI Command Set - environment commands.
> > + Copyright 2002 Free Software Foundation, Inc.
> > + Contributed by Red Hat Inc.
> > +
> > + 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 2 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, write to the Free Software
> > + Foundation, Inc., 59 Temple Place - Suite 330,
> > + Boston, MA 02111-1307, USA. */
> > +
> > +#include <string.h>
> > +#include <sys/stat.h>
> > +
> > +#include "defs.h"
> > +#include "target.h"
> > +#include "frame.h"
> > +#include "value.h"
> > +#include "mi-cmds.h"
> > +#include "mi-out.h"
> > +#include "ui-out.h"
> > +#include "symtab.h"
> > +#include "filenames.h"
> > +#include "environ.h"
> > +#include "command.h"
> > +#include "top.h"
> > +
> > +extern struct environ *inferior_environ;
> > +extern char *source_path;
> > +
> > +static void env_cli_command (const char *cli, char *args);
> > +static void env_mod_path (char *dirname, char **which_path);
> > +
> > +static const char path_var_name[] = "PATH";
> > +
> > +/* the following is copied from mi-main.c so for m1 and below we
> > + can perform old behavior and use cli commands */
> > +static void
> > +env_execute_cli_command (const char *cli, char *args)
> > +{
> > + if (cli != 0)
> > + {
> > + struct cleanup *old_cleanups;
> > + char *run;
> > + xasprintf (&run, cli, args);
> > + old_cleanups = make_cleanup (xfree, run);
> > + execute_command ( /*ui */ run, 0 /*from_tty */ );
> > + do_cleanups (old_cleanups);
> > + return;
> > + }
> > +}
> > +
> > +
> > +/* Print working directory. */
> > +enum mi_cmd_result
> > +mi_cmd_env_pwd (char *command, char **argv, int argc)
> > +{
> > + if (argc > 0)
> > + error ("mi_cmd_env_pwd: No arguments required");
> > +
> > + if (mi_version (uiout) < 2)
> > + {
> > + env_execute_cli_command ("pwd", NULL);
> > + return MI_CMD_DONE;
> > + }
> > +
> > + /* otherwise mi level 2 or higher */
> > +
> > + getcwd (gdb_dirbuf, sizeof (gdb_dirbuf));
> > + ui_out_field_string (uiout, "cwd", gdb_dirbuf);
> > +
> > + return MI_CMD_DONE;
> > +}
> > +
> > +/* Change working directory. */
> > +enum mi_cmd_result
> > +mi_cmd_env_cd (char *command, char **argv, int argc)
> > +{
> > + if (argc == 0 || argc > 1)
> > + error ("mi_cmd_env_cd: Usage DIRECTORY");
> > +
> > + env_execute_cli_command ("cd %s", argv[0]);
> > +
> > + return MI_CMD_DONE;
> > +}
> > +
> > +static void
> > +env_mod_path (char *dirname, char **which_path)
> > +{
> > + if (dirname == 0 || dirname[0] == '\0')
> > + return;
> > +
> > + /* call add_path with last arg 0 to indicate not to parse for separator chars */
> > + add_path (dirname, which_path, 0);
> > +}
> > +
> > +/* Add one or more directories to start of executable search path */
> > +enum mi_cmd_result
> > +mi_cmd_env_path (char *command, char **argv, int argc)
> > +{
> > + char *exec_path;
> > + char *env;
> > + int i;
> > +
> > + if (mi_version (uiout) < 2)
> > + {
> > + for (i = argc - 1; i >= 0; --i)
> > + env_execute_cli_command ("path %s", argv[i]);
> > + return MI_CMD_DONE;
> > + }
> > +
> > + /* otherwise mi level 2 or higher */
> > + dont_repeat ();
> > + env = get_in_environ (inferior_environ, path_var_name);
> > +
> > + /* Can be null if path is not set */
> > + if (!env)
> > + env = "";
> > + exec_path = xstrdup (env);
> > +
> > + for (i = argc - 1; i >= 0; --i)
> > + env_mod_path (argv[i], &exec_path);
> > +
> > + set_in_environ (inferior_environ, path_var_name, exec_path);
> > + xfree (exec_path);
> > + env = get_in_environ (inferior_environ, path_var_name);
> > + ui_out_field_string (uiout, "path", env);
> > +
> > + return MI_CMD_DONE;
> > +}
> > +
> > +/* Add zero or more directories to the front of the source path. */
> > +enum mi_cmd_result
> > +mi_cmd_env_dir (char *command, char **argv, int argc)
> > +{
> > + int i;
> > +
> > + dont_repeat ();
> > +
> > + if (mi_version (uiout) < 2)
> > + {
> > + for (i = argc - 1; i >= 0; --i)
> > + env_execute_cli_command ("dir %s", argv[i]);
> > + return MI_CMD_DONE;
> > + }
> > +
> > + /* otherwise mi 2 or higher */
> > + if (argc == 0)
> > + {
> > + /* no args implies reset to default path */
> > + xfree (source_path);
> > + init_source_path ();
> > + }
> > + else
> > + {
> > + for (i = argc - 1; i >= 0; --i)
> > + env_mod_path (argv[i], &source_path);
> > + init_last_source_visited ();
> > + }
> > +
> > + ui_out_field_string (uiout, "source-path", source_path);
> > + forget_cached_source_info ();
> > +}
> > +
> > Index: mi/mi-cmds.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/mi/mi-cmds.c,v
> > retrieving revision 1.8
> > diff -u -r1.8 mi-cmds.c
> > --- mi/mi-cmds.c 6 Mar 2001 08:21:45 -0000 1.8
> > +++ mi/mi-cmds.c 7 Nov 2002 21:47:04 -0000
> > @@ -56,10 +56,10 @@
> > {"display-enable", 0, 0},
> > {"display-insert", 0, 0},
> > {"display-list", 0, 0},
> > - {"environment-cd", "cd %s", 0},
> > - {"environment-directory", "dir %s", 0},
> > - {"environment-path", "path %s", 0},
> > - {"environment-pwd", "pwd", 0},
> > + {"environment-cd", 0, 0, mi_cmd_env_cd},
> > + {"environment-directory", 0, 0, mi_cmd_env_dir},
> > + {"environment-path", 0, 0, mi_cmd_env_path},
> > + {"environment-pwd", 0, 0, mi_cmd_env_pwd},
> > {"exec-abort", 0, 0},
> > {"exec-arguments", "set args %s", 0},
> > {"exec-continue", 0, mi_cmd_exec_continue},
> > Index: mi/mi-cmds.h
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/mi/mi-cmds.h,v
> > retrieving revision 1.5
> > diff -u -r1.5 mi-cmds.h
> > --- mi/mi-cmds.h 6 Mar 2001 08:21:45 -0000 1.5
> > +++ mi/mi-cmds.h 7 Nov 2002 21:47:04 -0000
> > @@ -64,6 +64,10 @@
> > extern mi_cmd_argv_ftype mi_cmd_data_read_memory;
> > extern mi_cmd_argv_ftype mi_cmd_data_write_memory;
> > extern mi_cmd_argv_ftype mi_cmd_data_write_register_values;
> > +extern mi_cmd_argv_ftype mi_cmd_env_cd;
> > +extern mi_cmd_argv_ftype mi_cmd_env_dir;
> > +extern mi_cmd_argv_ftype mi_cmd_env_path;
> > +extern mi_cmd_argv_ftype mi_cmd_env_pwd;
> > extern mi_cmd_args_ftype mi_cmd_exec_continue;
> > extern mi_cmd_args_ftype mi_cmd_exec_finish;
> > extern mi_cmd_args_ftype mi_cmd_exec_next;
> > Index: gdbmi.texinfo
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/mi/gdbmi.texinfo,v
> > retrieving revision 1.29
> > diff -u -r1.29 gdbmi.texinfo
> > --- gdbmi.texinfo 3 Oct 2002 22:31:31 -0000 1.29
> > +++ gdbmi.texinfo 7 Nov 2002 21:55:40 -0000
> > @@ -1665,10 +1665,12 @@
> > @subsubheading Synopsis
> >
> > @example
> > - -environment-directory @var{pathdir}
> > + -environment-directory [ @var{pathdir} ]+
> > @end example
> >
> > -Add directory @var{pathdir} to beginning of search path for source files.
> > +Add directories @var{pathdir} to beginning of search path for source files.
> > +If no argument is given, reset search path to default. An empty string for
> > +@var{pathdir} is ignored so it may be used to display the current search path.
> >
> > @subsubheading @value{GDBN} Command
> >
> > @@ -1679,7 +1681,13 @@
> > @smallexample
> > (@value{GDBP})
> > -environment-directory /kwikemart/marge/ezannoni/flathead-dev/devo/gdb
> > -^done
> > +^done,source-path="/kwikemart/marge/ezannoni/flathead-dev/devo/gdb:$cdir:$cwd"
> > +(@value{GDBP})
> > +-environment-directory ""
> > +^done,source-path="/kwikemart/marge/ezannoni/flathead-dev/devo/gdb:$cdir:$cwd"
> > +(@value{GDBP})
> > +-environment-directory
> > +^done,source-path="$cdir:$cwd"
> > (@value{GDBP})
> > @end smallexample
> >
> > @@ -1690,10 +1698,12 @@
> > @subsubheading Synopsis
> >
> > @example
> > - -environment-path ( @var{pathdir} )+
> > + -environment-path [ @var{pathdir} ]+
> > @end example
> >
> > Add directories @var{pathdir} to beginning of search path for object files.
> > +If no paths or an empty path is specified, the current object search path
> > +is displayed with no modification.
> >
> > @subsubheading @value{GDBN} Command
> >
> > @@ -1704,7 +1714,10 @@
> > @smallexample
> > (@value{GDBP})
> > -environment-path /kwikemart/marge/ezannoni/flathead-dev/ppc-eabi/gdb
> > -^done
> > +^done,path="/kwikemart/marge/ezannoni/flathead-dev/ppc-eabi/gdb:/usr/bin"
> > +(@value{GDBP})
> > +-environment-path
> > +^done,path="/kwikemart/marge/ezannoni/flathead-dev/ppc-eabi/gdb:/usr/bin"
> > (@value{GDBP})
> > @end smallexample
> >
> > @@ -1729,8 +1742,7 @@
> > @smallexample
> > (@value{GDBP})
> > -environment-pwd
> > -~Working directory /kwikemart/marge/ezannoni/flathead-dev/devo/gdb.
> > -^done
> > +^done,cwd="/kwikemart/marge/ezannoni/flathead-dev/devo/gdb"
> > (@value{GDBP})
> > @end smallexample
> >
> > Index: mi-basics.exp
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-basics.exp,v
> > retrieving revision 1.6
> > diff -u -r1.6 mi-basics.exp
> > --- mi-basics.exp 27 Jun 2001 17:27:07 -0000 1.6
> > +++ mi-basics.exp 7 Nov 2002 21:56:00 -0000
> > @@ -150,24 +150,50 @@
> >
> > # Clear the search directories, then specify one to be searched
> > # Tests:
> > - # -environment-directory
> > # -environment-directory arg
> > + # -environment-directory empty-string
> > + # -environment-directory
> >
> > #exp_internal 1
> > - mi_gdb_test "202-environment-directory" \
> > - "\\\^done" \
> > + mi_gdb_test "202-environment-directory ${srcdir}/${subdir}" \
> > + "\\\^done,source-path=\"${srcdir}/${subdir}:\\\$cdir:\\\$cwd\"" \
> > + "environment-directory arg operation"
> > +
> > + mi_gdb_test "203-environment-directory \"\"" \
> > + "\\\^done,source-path=\"${srcdir}/${subdir}:\\\$cdir:\\\$cwd\"" \
> > + "environment-directory empty-string operation"
> > +
> > + mi_gdb_test "204-environment-directory" \
> > + "\\\^done,source-path=\"\\\$cdir:\\\$cwd\"" \
> > "environment-directory operation"
> >
> > - mi_gdb_test "203-environment-directory ${srcdir}/${subdir}" \
> > - "\\\^done" \
> > - "environment-directory arg operation"
> > #exp_internal 0
> > }
> >
> > +proc test_cwd_specification {} {
> > + global mi_gdb_prompt
> > + global objdir
> > + global subdir
> > +
> > + # Change the working directory, then print the current working directory
> > + # Tests:
> > + # -environment-cd ${objdir}
> > + # -environment-pwd
> > +
> > + mi_gdb_test "205-environment-cd ${objdir}" \
> > + "\\\^done" \
> > + "environment-cd arg operation"
> > +
> > + mi_gdb_test "206-environment-pwd" \
> > + "\\\^done,cwd=\"${objdir}\"" \
> > + "environment-pwd operation"
> > +}
> > +
> > if [test_mi_interpreter_selection] {
> > test_exec_and_symbol_mi_operatons
> > test_breakpoints_deletion
> > test_dir_specification
> > + test_cwd_specification
> > }
> >
> > mi_gdb_exit