[RFA 2/2] warn if "source" fails to open the file when from_tty == 0

[RFA 2/2] warn if "source" fails to open the file when from_tty == 0

[Joel Brobecker]

Hello,

Consider the following example:

    % gdb -q -batch -ex 'source nonexistant-file'
    [nothing]

One would have at least expected the debugger to warn about
not finding the file, similar to the error shown when using
a more interactive mode. Eg:

    (gdb) source nonexistant-file
    nonexistant-file: No such file or directory.

Not raising an error appears to be intentional, presumably in order
to prevent this situation from stoping the execution of a GDB script.
But the lack of at least a warning makes it harder for a user to
diagnose any issue, if the file was expected to be there and readable.

This patch adds a warning in that case:

    % gdb -q -batch -ex 'source nonexistant-file'
    warning: nonexistant-file: No such file or directory.

gdb/ChangeLog:

        * utils.h (perror_warning_with_name): Add declaration.
        * utils.c (perror_warning_with_name): New function.
        * cli/cli-cmds.c (source_script_with_search): Add call to
        perror_warning_with_name if from_tty is nul.

gdb/testsuite/ChangeLog:

        * gdb.base/source-nofile.gdb: New file.
        * gdb.base/source.exp: Add two tests verifying the behavior when
        the "source" command is given a non-existant filename.

Tested on x86_64-linux, Ok to commit?

Thanks,
-- 
Joel

---
 gdb/cli/cli-cmds.c                       |  7 +++++--
 gdb/testsuite/gdb.base/source-nofile.gdb | 22 ++++++++++++++++++++++
 gdb/testsuite/gdb.base/source.exp        |  9 +++++++++
 gdb/utils.c                              | 13 +++++++++++++
 gdb/utils.h                              |  2 ++
 5 files changed, 51 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/source-nofile.gdb

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 460b719..0ef325d 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -571,11 +571,14 @@ source_script_with_search (const char *file, int from_tty, int search_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.  */
+	 just emit a warning, rather than cause an error.  */
       if (from_tty)
 	perror_with_name (file);
       else
-	return;
+	{
+	  perror_warning_with_name (file);
+	  return;
+	}
     }
 
   old_cleanups = make_cleanup (xfree, full_path);
diff --git a/gdb/testsuite/gdb.base/source-nofile.gdb b/gdb/testsuite/gdb.base/source-nofile.gdb
new file mode 100644
index 0000000..e577a41
--- /dev/null
+++ b/gdb/testsuite/gdb.base/source-nofile.gdb
@@ -0,0 +1,22 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2013 Free Software Foundation, Inc.
+#
+# 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 3 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, see <http://www.gnu.org/licenses/>.
+
+source for-sure-nonexistant-file
+
+# Just print an message to confirm that failing to source the file
+# did not cause this script to abort.
+echo source error not fatal\n
diff --git a/gdb/testsuite/gdb.base/source.exp b/gdb/testsuite/gdb.base/source.exp
index 61fd221..62a6599 100644
--- a/gdb/testsuite/gdb.base/source.exp
+++ b/gdb/testsuite/gdb.base/source.exp
@@ -57,4 +57,13 @@ gdb_test "source -v -s ./source-test.gdb" \
     "echo test source options.*" \
     "source -v -s"
 
+# Test sourcing a non-existant file, both when the source command
+# comes from the a command entered at the GDB prompt, and when
+# it comes from a script being sourced.
+gdb_test "source for-sure-nonexistant-file" \
+         "for-sure-nonexistant-file: No such file or directory\."
+
+gdb_test "source source-nofile.gdb" \
+         "warning: for-sure-nonexistant-file: No such file or directory\.\[\r\n\]*source error not fatal"
+
 gdb_exit
diff --git a/gdb/utils.c b/gdb/utils.c
index 402fe8e..89c248c 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1006,6 +1006,19 @@ perror_with_name (const char *string)
   throw_perror_with_name (GENERIC_ERROR, string);
 }
 
+/* Same as perror_with_name except that it prints a warning instead
+   of throwing an error.  */
+
+void
+perror_warning_with_name (const char *string)
+{
+  char *combined;
+
+  combined = perror_string (string);
+  warning (_("%s"), combined);
+  xfree (combined);
+}
+
 /* Print the system error message for ERRCODE, and also mention STRING
    as the file name for which the error was encountered.  */
 
diff --git a/gdb/utils.h b/gdb/utils.h
index 3492f09..7ea0ff4 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -285,6 +285,8 @@ extern void throw_perror_with_name (enum errors errcode, const char *string)
   ATTRIBUTE_NORETURN;
 extern void perror_with_name (const char *) ATTRIBUTE_NORETURN;
 
+extern void perror_warning_with_name (const char *string);
+
 extern void print_sys_errmsg (const char *, int);
 \f
 /* Warnings and error messages.  */
-- 
1.8.1.2

[RFA 1/2] new function perror_string extracted out of throw_perror_with_name.

[Joel Brobecker]

Hello,

The main purpose of this patch is to extract the part of
throw_perror_with_name that computes a string providing the system
error message combined with a prefix string.  This will become useful
later on to provide a routine which prints a warning using that
perror_string, rather than throwing an error.

gdb/ChangeLog:

        * utils.c (perror_string): New function, extracted out of
        throw_perror_with_name.
        (throw_perror_with_name): Rework to use perror_string.

Tested on x86_64-linux, ok to commit?

Thanks,
-- 
Joel

---
 gdb/utils.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/gdb/utils.c b/gdb/utils.c
index 26879ec..402fe8e 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -957,6 +957,26 @@ add_internal_problem_command (struct internal_problem *problem)
   xfree (show_doc);
 }
 
+/* Return a newly allocated string, containing the PREFIX followed
+   by the system error message for errno (separated by a colon).
+
+   The result must be deallocated after use.  */
+
+static char *
+perror_string (const char *prefix)
+{
+  char *err;
+  char *combined;
+
+  err = safe_strerror (errno);
+  combined = (char *) xmalloc (strlen (err) + strlen (prefix) + 3);
+  strcpy (combined, prefix);
+  strcat (combined, ": ");
+  strcat (combined, err);
+
+  return combined;
+}
+
 /* Print the system error message for errno, and also mention STRING
    as the file name for which the error was encountered.  Use ERRCODE
    for the thrown exception.  Then return to command level.  */
@@ -964,14 +984,10 @@ add_internal_problem_command (struct internal_problem *problem)
 void
 throw_perror_with_name (enum errors errcode, const char *string)
 {
-  char *err;
   char *combined;
 
-  err = safe_strerror (errno);
-  combined = (char *) alloca (strlen (err) + strlen (string) + 3);
-  strcpy (combined, string);
-  strcat (combined, ": ");
-  strcat (combined, err);
+  combined = perror_string (string);
+  make_cleanup (xfree, combined);
 
   /* I understand setting these is a matter of taste.  Still, some people
      may clear errno but not know about bfd_error.  Doing this here is not
-- 
1.8.1.2

Re: [RFA 1/2] new function perror_string extracted out of throw_perror_with_name.

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> gdb/ChangeLog:

Joel>         * utils.c (perror_string): New function, extracted out of
Joel>         throw_perror_with_name.
Joel>         (throw_perror_with_name): Rework to use perror_string.

Joel> Tested on x86_64-linux, ok to commit?

Looks good.

Tom

Re: [RFA 2/2] warn if "source" fails to open the file when from_tty == 0

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> One would have at least expected the debugger to warn about
Joel> not finding the file, similar to the error shown when using
Joel> a more interactive mode. Eg:

Yeah.  That seems like a mistake, but it seems like it would also be a
mistake to change it.

Joel> Tested on x86_64-linux, Ok to commit?

Looks good to me.

Tom

Re: [RFA 2/2] warn if "source" fails to open the file when from_tty == 0

[Joel Brobecker]

> Joel> One would have at least expected the debugger to warn about
> Joel> not finding the file, similar to the error shown when using
> Joel> a more interactive mode. Eg:
> 
> Yeah.  That seems like a mistake, but it seems like it would also be a
> mistake to change it.

That's what I thought as well. Not sure whether anyone really relies
on that behavior, or not, but it's been like that at least since
start of CVS, as far as I can tell, so really hard to argue for
a change.

> Joel> Tested on x86_64-linux, Ok to commit?
> 
> Looks good to me.

Thanks! Checked in.

-- 
Joel

Re: [RFA 1/2] new function perror_string extracted out of throw_perror_with_name.

[Joel Brobecker]

> Joel>         * utils.c (perror_string): New function, extracted out of
> Joel>         throw_perror_with_name.
> Joel>         (throw_perror_with_name): Rework to use perror_string.
> 
> Joel> Tested on x86_64-linux, ok to commit?
> 
> Looks good.

Thank you! Checked in.

-- 
Joel