Re: [RFA] Add -s option to source command.

[Doug Evans <dje@google.com>]

[-- Attachment #1: Type: text/plain, Size: 3734 bytes --]

On Fri, Apr 9, 2010 at 12:31 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Fri, 9 Apr 2010 11:12:27 -0700
>> From: Doug Evans <dje@google.com>
>> Cc: tromey@redhat.com, gdb-patches@sourceware.org
>>
>> > This is fine, but what if @var{filename} is @file{d:/foo/myscript} (on
>> > Windows)?
>>
>> source.c:openp() doesn't handle that case, it just blindly concatenates.
>> [presumably because it hasn't needed to]
>>
>> I don't have an opinion on what *should* happen here.
>> Possibilities are to either not try or remove the drive spec.
>
> My vote is for removing the drive letter.  The other alternative means
> that absolute file names are handled inconsistently across platforms
> (I assume that not trying to look for absolute file name on Posix
> platforms will not be a useful behavior).

Hi.  Here is the patch I will check in pending approval of the doc
changes to mention dos drive spec handling.

2010-04-08  Doug Evans  <dje@google.com>

	Add -s option to source command.
	* NEWS: Document new option.
	* cli/cli-cmds.c (find_and_open_script): Add function comment.
	Delete from_tty and cleanupp args.  Split filep arg into file and
	full_pathp.  New arg search_path.
	(source_script_from_stream): New function.
	(source_script_with_search): New function.
	(source_script): Rewrite.
	(source_command): Parse "-s" option.
	(init_cli_cmds): Add "-s" docs to source command help, and reformat.
	* python/python.c (source_python_script): Make file arg a const char *.
	Don't call fclose, leave for caller.
	* python/python.h (source_python_script): Update.

	testsuite/
	* gdb.base/source-test.gdb: New file.
	* gdb.base/source.exp: Add tests for "source -s".

	doc/
	* gdb.texinfo (Command Files): Add docs for new "source -s" option.

Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.704
diff -u -p -r1.704 gdb.texinfo
--- doc/gdb.texinfo	14 Apr 2010 12:02:44 -0000	1.704
+++ doc/gdb.texinfo	14 Apr 2010 21:54:37 -0000
@@ -19373,7 +19373,7 @@ using the @code{script-extension} settin
 @table @code
 @kindex source
 @cindex execute commands from a file
-@item source [@code{-v}] @var{filename}
+@item source [-s] [-v] @var{filename}
 Execute the command file @var{filename}.
 @end table

@@ -19390,6 +19390,21 @@ directory, then @value{GDBN} also looks
 except that @file{$cdir} is not searched because the compilation directory
 is not relevant to scripts.

+If @code{-s} is specified, then @value{GDBN} searches for @var{filename}
+on the search path even if @var{filename} specifies a directory.
+The search is done by appending @var{filename} to each element of the
+search path.  So, for example, if @var{filename} is @file{mylib/myscript}
+and the search path contains @file{/home/user} then @value{GDBN} will
+look for the script @file{/home/user/mylib/myscript}.
+The search is also done if @var{filename} is an absolute path.
+For example, if @var{filename} is @file{/tmp/myscript} and
+the search path contains @file{/home/user} then @value{GDBN} will
+look for the script @file{/home/user/tmp/myscript}.
+For DOS-like systems, if @var{filename} contains a drive specification,
+it is stripped before concatenation.  For example, if @var{filename} is
+@file{d:myscript} and the search path contains @file{c:/tmp} then @value{GDBN}
+will look for the script @file{c:/tmp/myscript}.
+
 If @code{-v}, for verbose mode, is given then @value{GDBN} displays
 each command as it is executed.  The option must be given before
 @var{filename}, and is interpreted as part of the filename anywhere else.


Full patch attached.

[-- Attachment #2: gdb-100414-source-s-3.patch.txt --]
[-- Type: text/plain, Size: 13307 bytes --]

2010-04-08  Doug Evans  <dje@google.com>

	Add -s option to source command.
	* NEWS: Document new option.
	* cli/cli-cmds.c (find_and_open_script): Add function comment.
	Delete from_tty and cleanupp args.  Split filep arg into file and
	full_pathp.  New arg search_path.
	(source_script_from_stream): New function.
	(source_script_with_search): New function.
	(source_script): Rewrite.
	(source_command): Parse "-s" option.
	(init_cli_cmds): Add "-s" docs to source command help, and reformat.
	* python/python.c (source_python_script): Make file arg a const char *.
	Don't call fclose, leave for caller.
	* python/python.h (source_python_script): Update.

	testsuite/
	* gdb.base/source-test.gdb: New file.
	* gdb.base/source.exp: Add tests for "source -s".

	doc/
	* gdb.texinfo (Command Files): Add docs for new "source -s" option.

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.369
diff -u -p -r1.369 NEWS
--- NEWS	9 Apr 2010 15:26:54 -0000	1.369
+++ NEWS	14 Apr 2010 21:54:37 -0000
@@ -3,6 +3,10 @@
 
 *** Changes since GDB 7.1
 
+* The source command now accepts a -s option to force searching for the
+  script in the source search path even if the script name specifies
+  a directory.
+
 * New features in the GDB remote stub, GDBserver
 
   - GDBserver now support tracepoints.  The feature is currently
Index: cli/cli-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.100
diff -u -p -r1.100 cli-cmds.c
--- cli/cli-cmds.c	9 Apr 2010 03:00:58 -0000	1.100
+++ cli/cli-cmds.c	14 Apr 2010 21:54:37 -0000
@@ -470,62 +470,59 @@ Script filename extension recognition is
 		    value);
 }
 
+/* Try to open SCRIPT_FILE.
+   If successful, the full path name is stored in *FULL_PATHP,
+   the stream is stored in *STREAMP, and return 1.
+   The caller is responsible for freeing *FULL_PATHP.
+   If not successful, return 0; errno is set for the last file
+   we tried to open.
+
+   If SEARCH_PATH is non-zero, and the file isn't found in cwd,
+   search for it in the source search path.
+
+   NOTE: This calls openp which uses xfullpath to compute the full path
+   instead of gdb_realpath.  Symbolic links are not resolved.  */
+
 static int
-find_and_open_script (int from_tty, char **filep, FILE **streamp,
-		      struct cleanup **cleanupp)
+find_and_open_script (const char *script_file, int search_path,
+		      FILE **streamp, char **full_pathp)
 {
-  char *file = *filep;
-  char *full_pathname = NULL;
+  char *file;
   int fd;
   struct cleanup *old_cleanups;
+  int search_flags = OPF_TRY_CWD_FIRST;
 
-  file = tilde_expand (file);
+  file = tilde_expand (script_file);
   old_cleanups = make_cleanup (xfree, file);
 
-  /* Search for and open 'file' on the search path used for source
-     files.  Put the full location in 'full_pathname'.  */
-  fd = openp (source_path, OPF_TRY_CWD_FIRST,
-	      file, O_RDONLY, &full_pathname);
-  make_cleanup (xfree, full_pathname);
+  if (search_path)
+    search_flags |= OPF_SEARCH_IN_PATH;
 
-  /* Use the full path name, if it is found.  */
-  if (full_pathname != NULL && fd != -1)
-    {
-      file = full_pathname;
-    }
+  /* Search for and open 'file' on the search path used for source
+     files.  Put the full location in *FULL_PATHP.  */
+  fd = openp (source_path, search_flags,
+	      file, O_RDONLY, full_pathp);
 
   if (fd == -1)
     {
-      if (from_tty)
-	perror_with_name (file);
-      else
-	{
-	  do_cleanups (old_cleanups);
-	  return 0;
-	}
+      int save_errno = errno;
+      do_cleanups (old_cleanups);
+      errno = save_errno;
+      return 0;
     }
 
-  *streamp = fdopen (fd, FOPEN_RT);
-  *filep = file;
-  *cleanupp = old_cleanups;
+  do_cleanups (old_cleanups);
 
+  *streamp = fdopen (fd, FOPEN_RT);
   return 1;
 }
 
-void
-source_script (char *file, int from_tty)
-{
-  FILE *stream;
-  struct cleanup *old_cleanups;
-
-  if (file == NULL || *file == 0)
-    {
-      error (_("source command requires file name of file to source."));
-    }
-
-  if (!find_and_open_script (from_tty, &file, &stream, &old_cleanups))
-    return;
+/* Load script FILE, which has already been opened as STREAM.
+   STREAM is closed before we return.  */
 
+static void
+source_script_from_stream (FILE *stream, const char *file)
+{
   if (script_ext_mode != script_ext_off
       && strlen (file) > 3 && !strcmp (&file[strlen (file) - 3], ".py"))
     {
@@ -541,22 +538,64 @@ source_script (char *file, int from_tty)
 	  if (script_ext_mode == script_ext_soft
 	      && e.reason == RETURN_ERROR && e.error == UNSUPPORTED_ERROR)
 	    {
-	      if (!find_and_open_script (from_tty, &file, &stream, &old_cleanups))
-		return;
-
-	      script_from_file (stream, file);
+	      fseek (stream, 0, SEEK_SET);
+	      script_from_file (stream, (char*) file);
 	    }
 	  else
-	    /* Nope, just punt.  */
-	    throw_exception (e);
+	    {
+	      /* Nope, just punt.  */
+	      fclose (stream);
+	      throw_exception (e);
+	    }
 	}
+      else
+	fclose (stream);
     }
   else
     script_from_file (stream, file);
+}
 
+/* Worker to perform the "source" command.
+   Load script FILE.
+   If SEARCH_PATH is non-zero, and the file isn't found in cwd,
+   search for it in the source search path.  */
+
+static void
+source_script_with_search (const char *file, int from_tty, int search_path)
+{
+  FILE *stream;
+  char *full_path;
+  struct cleanup *old_cleanups;
+
+  if (file == NULL || *file == 0)
+    error (_("source command requires file name of file to source."));
+
+  if (!find_and_open_script (file, search_path, &stream, &full_path))
+    {
+      /* The script wasn't found, or was otherwise inaccessible.
+	 If the source command was invoked interactively, throw an error.
+	 Otherwise (e.g. if it was invoked by a script), silently ignore
+	 the error.  */
+      if (from_tty)
+	perror_with_name (file);
+      else
+	return;
+    }
+
+  old_cleanups = make_cleanup (xfree, full_path);
+  source_script_from_stream (stream, file);
   do_cleanups (old_cleanups);
 }
 
+/* Wrapper around source_script_with_search to export it to main.c
+   for use in loading .gdbinit scripts.  */
+
+void
+source_script (char *file, int from_tty)
+{
+  source_script_with_search (file, from_tty, 0);
+}
+
 /* Return the source_verbose global variable to its previous state
    on exit from the source command, by whatever means.  */
 static void
@@ -572,33 +611,52 @@ source_command (char *args, int from_tty
   struct cleanup *old_cleanups;
   char *file = args;
   int *old_source_verbose = xmalloc (sizeof(int));
+  int search_path = 0;
 
   *old_source_verbose = source_verbose;
   old_cleanups = make_cleanup (source_verbose_cleanup, old_source_verbose);
 
   /* -v causes the source command to run in verbose mode.
+     -s causes the file to be searched in the source search path,
+     even if the file name contains a '/'.
      We still have to be able to handle filenames with spaces in a
      backward compatible way, so buildargv is not appropriate.  */
 
   if (args)
     {
-      /* Make sure leading white space does not break the comparisons.  */
-      while (isspace(args[0]))
-	args++;
-
-      /* Is -v the first thing in the string?  */
-      if (args[0] == '-' && args[1] == 'v' && isspace (args[2]))
+      while (args[0] != '\0')
 	{
-	  source_verbose = 1;
+	  /* Make sure leading white space does not break the comparisons.  */
+	  while (isspace(args[0]))
+	    args++;
+
+	  if (args[0] != '-')
+	    break;
+
+	  if (args[1] == 'v' && isspace (args[2]))
+	    {
+	      source_verbose = 1;
+
+	      /* Skip passed -v.  */
+	      args = &args[3];
+	    }
+	  else if (args[1] == 's' && isspace (args[2]))
+	    {
+	      search_path = 1;
 
-	  /* Trim -v and whitespace from the filename.  */
-	  file = &args[3];
-	  while (isspace (file[0]))
-	    file++;
+	      /* Skip passed -s.  */
+	      args = &args[3];
+	    }
+	  else
+	    break;
 	}
+
+      while (isspace (args[0]))
+	args++;
+      file = args;
     }
 
-  source_script (file, from_tty);
+  source_script_with_search (file, from_tty, search_path);
 
   do_cleanups (old_cleanups);
 }
@@ -1379,8 +1437,12 @@ Commands defined in this way may have up
 
   source_help_text = xstrprintf (_("\
 Read commands from a file named FILE.\n\
-Optional -v switch (before the filename) causes each command in\n\
-FILE to be echoed as it is executed.\n\
+\n\
+Usage: source [-s] [-v] FILE\n\
+-s: search for the script in the source search path,\n\
+    even if FILE contains directories.\n\
+-v: each command in FILE is echoed as it is executed.\n\
+\n\
 Note that the file \"%s\" is read automatically in this way\n\
 when GDB is started."), gdbinit);
   c = add_cmd ("source", class_support, source_command,
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.704
diff -u -p -r1.704 gdb.texinfo
--- doc/gdb.texinfo	14 Apr 2010 12:02:44 -0000	1.704
+++ doc/gdb.texinfo	14 Apr 2010 21:54:37 -0000
@@ -19373,7 +19373,7 @@ using the @code{script-extension} settin
 @table @code
 @kindex source
 @cindex execute commands from a file
-@item source [@code{-v}] @var{filename}
+@item source [-s] [-v] @var{filename}
 Execute the command file @var{filename}.
 @end table
 
@@ -19390,6 +19390,21 @@ directory, then @value{GDBN} also looks 
 except that @file{$cdir} is not searched because the compilation directory
 is not relevant to scripts.
 
+If @code{-s} is specified, then @value{GDBN} searches for @var{filename}
+on the search path even if @var{filename} specifies a directory.
+The search is done by appending @var{filename} to each element of the
+search path.  So, for example, if @var{filename} is @file{mylib/myscript}
+and the search path contains @file{/home/user} then @value{GDBN} will
+look for the script @file{/home/user/mylib/myscript}.
+The search is also done if @var{filename} is an absolute path.
+For example, if @var{filename} is @file{/tmp/myscript} and
+the search path contains @file{/home/user} then @value{GDBN} will
+look for the script @file{/home/user/tmp/myscript}.
+For DOS-like systems, if @var{filename} contains a drive specification,
+it is stripped before concatenation.  For example, if @var{filename} is
+@file{d:myscript} and the search path contains @file{c:/tmp} then @value{GDBN}
+will look for the script @file{c:/tmp/myscript}.
+
 If @code{-v}, for verbose mode, is given then @value{GDBN} displays
 each command as it is executed.  The option must be given before
 @var{filename}, and is interpreted as part of the filename anywhere else.
Index: python/python.c
===================================================================
RCS file: /cvs/src/src/gdb/python/python.c,v
retrieving revision 1.30
diff -u -p -r1.30 python.c
--- python/python.c	14 Apr 2010 13:18:55 -0000	1.30
+++ python/python.c	14 Apr 2010 21:54:37 -0000
@@ -364,10 +364,11 @@ gdbpy_parse_and_eval (PyObject *self, Py
 }
 
 /* Read a file as Python code.  STREAM is the input file; FILE is the
-   name of the file.  */
+   name of the file.
+   STREAM is not closed, that is the caller's responsibility.  */
 
 void
-source_python_script (FILE *stream, char *file)
+source_python_script (FILE *stream, const char *file)
 {
   struct cleanup *cleanup;
 
@@ -375,7 +376,6 @@ source_python_script (FILE *stream, char
 
   PyRun_SimpleFile (stream, file);
 
-  fclose (stream);
   do_cleanups (cleanup);
 }
 
@@ -562,9 +562,8 @@ eval_python_from_control_command (struct
 }
 
 void
-source_python_script (FILE *stream, char *file)
+source_python_script (FILE *stream, const char *file)
 {
-  fclose (stream);
   throw_error (UNSUPPORTED_ERROR,
 	       _("Python scripting is not supported in this copy of GDB."));
 }
Index: python/python.h
===================================================================
RCS file: /cvs/src/src/gdb/python/python.h,v
retrieving revision 1.7
diff -u -p -r1.7 python.h
--- python/python.h	18 Jan 2010 06:25:22 -0000	1.7
+++ python/python.h	14 Apr 2010 21:54:37 -0000
@@ -24,7 +24,7 @@
 
 void eval_python_from_control_command (struct command_line *);
 
-void source_python_script (FILE *stream, char *file);
+void source_python_script (FILE *stream, const char *file);
 
 int apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
 			      int embedded_offset, CORE_ADDR address,
Index: testsuite/gdb.base/source.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/source.exp,v
retrieving revision 1.11
diff -u -p -r1.11 source.exp
--- testsuite/gdb.base/source.exp	7 Apr 2010 03:47:22 -0000	1.11
+++ testsuite/gdb.base/source.exp	14 Apr 2010 21:54:37 -0000
@@ -51,4 +51,17 @@ gdb_test_multiple "source ${srcdir}/${su
     }
 }
 
+gdb_test "dir ${srcdir}/${subdir}" ""
+gdb_test "source -s ./source-test.gdb" \
+    "test source options" \
+    "source -s"
+
+# Test -v and -s in either order.
+gdb_test "source -s -v ./source-test.gdb" \
+    "echo test source options.*" \
+    "source -s -v"
+gdb_test "source -v -s ./source-test.gdb" \
+    "echo test source options.*" \
+    "source -v -s"
+
 gdb_exit