From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19063 invoked by alias); 8 Nov 2002 23:53:38 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 19007 invoked from network); 8 Nov 2002 23:53:37 -0000 Received: from unknown (HELO touchme.toronto.redhat.com) (216.129.200.2) by sources.redhat.com with SMTP; 8 Nov 2002 23:53:37 -0000 Received: from redhat.com (toocool.toronto.redhat.com [172.16.14.72]) by touchme.toronto.redhat.com (Postfix) with ESMTP id C585C800019; Fri, 8 Nov 2002 18:53:36 -0500 (EST) Message-ID: <3DCC4E80.D1688178@redhat.com> Date: Fri, 08 Nov 2002 15:53:00 -0000 From: "J. Johnston" Organization: Red Hat Inc. X-Accept-Language: en MIME-Version: 1.0 To: Elena Zannoni Cc: gdb-patches@sources.redhat.com Subject: Re: Proposed patch for gdb/mi 741 References: <3DA4886F.194D484D@redhat.com> <15797.30559.329315.574728@localhost.redhat.com> <3DCAE4E2.D05B3DD8@redhat.com> <15818.64935.170977.501640@localhost.redhat.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-SW-Source: 2002-11/txt/msg00264.txt.bz2 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 > > > > * 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 > > > > * 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 > > > > * 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 > > +#include > > + > > +#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