[PATCH] gdb/guile: perform tilde expansion when sourcing guile scripts

[PATCH] gdb/guile: perform tilde expansion when sourcing guile scripts

[Andrew Burgess]

Before this patch:

  (gdb) source ~/script.scm
  ERROR: In procedure apply-smob/1:
  ERROR: In procedure primitive-load-path: Unable to find file "~/script.scm" in load path
  Error while executing Scheme code.
  (gdb)

This is because the path is not tilde expanded.  In contrast, when
sourcing a .py or .gdb script the path is tilde expanded.

This commit fixes this oversight, and allows the above source command
to work as expected.

While I was working on the function that performs the sourcing I added
a use of gdb::unique_xmalloc_ptr<char>, this allows me to remove some
manual memory management.

gdb/ChangeLog:

	* guile/guile.c: Add 'gdbsupport/gdb_tilde_expand.h' include.
	(gdbscm_source_script): Expand filename using gdb_tilde_expand,
	and wrap error message in gdb::unique_xmalloc_ptr<char> to avoid
	manual memory management.

gdb/testsuite/ChangeLog:

	* gdb.guile/guile.exp: Add an extra test.
---
 gdb/ChangeLog                     |  7 +++++++
 gdb/guile/guile.c                 | 11 ++++++-----
 gdb/testsuite/ChangeLog           |  4 ++++
 gdb/testsuite/gdb.guile/guile.exp |  8 ++++++++
 4 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/gdb/guile/guile.c b/gdb/guile/guile.c
index c6959f5b713..d91427f4536 100644
--- a/gdb/guile/guile.c
+++ b/gdb/guile/guile.c
@@ -37,6 +37,7 @@
 #endif
 #include <signal.h>
 #include "gdbsupport/block-signals.h"
+#include "gdbsupport/gdb_tilde_expand.h"
 
 /* The Guile version we're using.
    We *could* use the macros in libguile/version.h but that would preclude
@@ -270,13 +271,13 @@ static void
 gdbscm_source_script (const struct extension_language_defn *extlang,
 		      FILE *file, const char *filename)
 {
-  char *msg = gdbscm_safe_source_script (filename);
+  std::string full_path = gdb_tilde_expand (filename);
+
+  gdb::unique_xmalloc_ptr<char> msg
+    (gdbscm_safe_source_script (full_path.c_str ()));
 
   if (msg != NULL)
-    {
-      fprintf_filtered (gdb_stderr, "%s\n", msg);
-      xfree (msg);
-    }
+    fprintf_filtered (gdb_stderr, "%s\n", msg.get ());
 }
 \f
 /* (execute string [#:from-tty boolean] [#:to-string boolean])
diff --git a/gdb/testsuite/gdb.guile/guile.exp b/gdb/testsuite/gdb.guile/guile.exp
index 6e464cc0e77..06b7c006ee6 100644
--- a/gdb/testsuite/gdb.guile/guile.exp
+++ b/gdb/testsuite/gdb.guile/guile.exp
@@ -82,3 +82,11 @@ gdb_test_no_output "guile (define a (execute \"help\" #:to-string #t))" \
 
 gdb_test "guile (print a)" "= .*aliases -- User-defined aliases of other commands.*" \
     "verify help to uiout"
+
+# Verify that we can source a guile script using ~ for the HOME directory.
+
+save_vars { env(HOME) } {
+    set env(HOME) $srcdir/$subdir
+    clean_restart
+    gdb_test "source ~/source2.scm" "yes"
+}
-- 
2.25.4

Re: [PATCH] gdb/guile: perform tilde expansion when sourcing guile scripts

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> This is because the path is not tilde expanded.  In contrast, when
Andrew> sourcing a .py or .gdb script the path is tilde expanded.

Andrew> This commit fixes this oversight, and allows the above source command
Andrew> to work as expected.

It seems a little strange to me that each extension language must do
this, rather than the caller doing it.

Andrew> +  gdb::unique_xmalloc_ptr<char> msg
Andrew> +    (gdbscm_safe_source_script (full_path.c_str ()));

I suppose gdbscm_safe_source_script could return a unique_xmalloc_ptr,
and this could just use the '=' form.

Tom

[PATCHv2 0/2] gdb/guile: perform tilde expansion when sourcing guile scripts

[Andrew Burgess]

* Tom Tromey <tom@tromey.com> [2021-05-05 13:11:37 -0600]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew> This is because the path is not tilde expanded.  In contrast, when
> Andrew> sourcing a .py or .gdb script the path is tilde expanded.
> 
> Andrew> This commit fixes this oversight, and allows the above source command
> Andrew> to work as expected.
> 
> It seems a little strange to me that each extension language must do
> this, rather than the caller doing it.

I believe that this is the right solution though.  I have updated the
commit message (in patch #1) to better explain why I think this.  If
you still disagree, just let me know and I'll move the logic.

> 
> Andrew> +  gdb::unique_xmalloc_ptr<char> msg
> Andrew> +    (gdbscm_safe_source_script (full_path.c_str ()));
> 
> I suppose gdbscm_safe_source_script could return a unique_xmalloc_ptr,
> and this could just use the '=' form.

That would be better, then all of the users would be forced to change
(which would be a good thing).  I span this out into patch #2.

Thanks,
Andrew

---

Andrew Burgess (2):
  gdb/guile: perform tilde expansion when sourcing guile scripts
  gdb/guile: Have gdbscm_safe_source_script return a unique_ptr

 gdb/ChangeLog                     | 17 +++++++++++++++++
 gdb/guile/guile-internal.h        |  3 ++-
 gdb/guile/guile.c                 |  7 ++-----
 gdb/guile/scm-objfile.c           |  9 ++-------
 gdb/guile/scm-safe-call.c         |  8 +++++---
 gdb/testsuite/ChangeLog           |  4 ++++
 gdb/testsuite/gdb.guile/guile.exp |  7 +++++++
 7 files changed, 39 insertions(+), 16 deletions(-)

-- 
2.25.4

[PATCHv2 1/2] gdb/guile: perform tilde expansion when sourcing guile scripts

[Andrew Burgess]

Before this patch:

  (gdb) source ~/script.scm
  ERROR: In procedure apply-smob/1:
  ERROR: In procedure primitive-load-path: Unable to find file "~/script.scm" in load path
  Error while executing Scheme code.
  (gdb)

This is because the path is not tilde expanded.  In contrast, when
sourcing a .py or .gdb script the path is tilde expanded.

This commit fixes this oversight, and allows the above source command
to work as expected.

The tilde expansion is done in guile/scm-safe-call.c rather than at a
higher level, for example, it might be tempting to add the tilde
expansion in cli/cli-cmds.c:source_script_from_stream, however, not
every extension language wants to see a tilde expanded file path.  For
example, consider Python.  Currently we pass in an unexpanded path and
we see this behaviour:

  (gdb) source ~/tmp/xxx.py
  Traceback (most recent call last):
    File "~/tmp/xxx.py", line 1, in <module>
  NameError: name 'undefined' is not defined

If we performed tilde expansion prior to calling the Python sourcer
function (gdbpy_source_script), we would instead see:

  (gdb) source ~/tmp/xxx.py
  Traceback (most recent call last):
    File "/home/andrew/tmp/xxx.py", line 1, in <module>
      undefined
  NameError: name 'undefined' is not defined

Notice the path expansion in the second line of the error message, I
believe this is a change for the worse.

And so, I think the best solution is for each language to perform its
own tilde expansion if this is required, i.e. if the underlying
language engine does not already handle tilde expansion.

gdb/ChangeLog:

	* guile/scm-safe-call.c: Add 'gdbsupport/gdb_tilde_expand.h'
	include.
	(gdbscm_safe_source_script): Perform tilde expansion if the path
	is not absolute.

gdb/testsuite/ChangeLog:

	* gdb.guile/guile.exp: Add an extra test.
---
 gdb/ChangeLog                     | 7 +++++++
 gdb/guile/scm-safe-call.c         | 4 +++-
 gdb/testsuite/ChangeLog           | 4 ++++
 gdb/testsuite/gdb.guile/guile.exp | 7 +++++++
 4 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/gdb/guile/scm-safe-call.c b/gdb/guile/scm-safe-call.c
index 2a38d6331fb..5bea970239b 100644
--- a/gdb/guile/scm-safe-call.c
+++ b/gdb/guile/scm-safe-call.c
@@ -24,6 +24,7 @@
 #include "filenames.h"
 #include "guile-internal.h"
 #include "gdbsupport/pathstuff.h"
+#include "gdbsupport/gdb_tilde_expand.h"
 
 /* Struct to marshall args to scscm_safe_call_body.  */
 
@@ -444,7 +445,8 @@ gdbscm_safe_source_script (const char *filename)
 
   if (!IS_ABSOLUTE_PATH (filename))
     {
-      abs_filename = gdb_realpath (filename);
+      abs_filename = gdb_tilde_expand_up (filename);
+      abs_filename = gdb_realpath (abs_filename.get ());
       filename = abs_filename.get ();
     }
 
diff --git a/gdb/testsuite/gdb.guile/guile.exp b/gdb/testsuite/gdb.guile/guile.exp
index 6e464cc0e77..0fb82284f46 100644
--- a/gdb/testsuite/gdb.guile/guile.exp
+++ b/gdb/testsuite/gdb.guile/guile.exp
@@ -82,3 +82,10 @@ gdb_test_no_output "guile (define a (execute \"help\" #:to-string #t))" \
 
 gdb_test "guile (print a)" "= .*aliases -- User-defined aliases of other commands.*" \
     "verify help to uiout"
+
+# Verify that we can source a guile script using ~ for the HOME directory.
+save_vars { env(HOME) } {
+    set env(HOME) $srcdir/$subdir
+    clean_restart
+    gdb_test "source ~/source2.scm" "yes"
+}
-- 
2.25.4

Re: [PATCHv2 1/2] gdb/guile: perform tilde expansion when sourcing guile scripts

[Simon Marchi]

On 2021-05-05 5:01 p.m., Andrew Burgess wrote:
> Before this patch:
> 
>   (gdb) source ~/script.scm
>   ERROR: In procedure apply-smob/1:
>   ERROR: In procedure primitive-load-path: Unable to find file "~/script.scm" in load path
>   Error while executing Scheme code.
>   (gdb)
> 
> This is because the path is not tilde expanded.  In contrast, when
> sourcing a .py or .gdb script the path is tilde expanded.
> 
> This commit fixes this oversight, and allows the above source command
> to work as expected.
> 
> The tilde expansion is done in guile/scm-safe-call.c rather than at a
> higher level, for example, it might be tempting to add the tilde
> expansion in cli/cli-cmds.c:source_script_from_stream, however, not
> every extension language wants to see a tilde expanded file path.  For
> example, consider Python.  Currently we pass in an unexpanded path and
> we see this behaviour:
> 
>   (gdb) source ~/tmp/xxx.py
>   Traceback (most recent call last):
>     File "~/tmp/xxx.py", line 1, in <module>
>   NameError: name 'undefined' is not defined
> 
> If we performed tilde expansion prior to calling the Python sourcer
> function (gdbpy_source_script), we would instead see:
> 
>   (gdb) source ~/tmp/xxx.py
>   Traceback (most recent call last):
>     File "/home/andrew/tmp/xxx.py", line 1, in <module>
>       undefined
>   NameError: name 'undefined' is not defined
> 
> Notice the path expansion in the second line of the error message, I
> believe this is a change for the worse.

I really don't have a strong opinion, but I don't think this would be a
problematic change.  The tilde-expansion thing is really a shell thing
(and we replicate it in GDB).  But once the paths are processed by
programs such as Python, the tilde usually doesn't have a special
meaning.

If I do:

    $ python3 ~/foo.py

I get:

    Traceback (most recent call last):
      File "/home/simark/foo.py", line 1, in <module>
	blah
    NameError: name 'blah' is not defined

Because the shell did the tilde-expansion before passing the file name
to Python.  If I do:

    $ python3 '~/foo.py'

then I get:

    python3: can't open file '/home/simark/build/binutils-gdb-one-target/gdb/~/foo.py': [Errno 2] No such file or directory

Python shows the tilde version of the path in our error messages just
becore we tell it that's the name of the file, Python doesn't use that
name otherwise (I think).  We could tell it the filename is "potato",
and it would say that the error is in file "potato".

So in a sense, passing the tilde-expanded version would make it look
more like the "regular" Python interpreter.

But like I said, I don't have a strong opinion: in the end this is shown
to the user, they will understand it either way.

Otherwise, the patch LGTM.

Simon

Re: [PATCHv2 1/2] gdb/guile: perform tilde expansion when sourcing guile scripts

[Andrew Burgess]

* Simon Marchi <simon.marchi@polymtl.ca> [2021-05-05 22:23:11 -0400]:

> 
> 
> On 2021-05-05 5:01 p.m., Andrew Burgess wrote:
> > Before this patch:
> > 
> >   (gdb) source ~/script.scm
> >   ERROR: In procedure apply-smob/1:
> >   ERROR: In procedure primitive-load-path: Unable to find file "~/script.scm" in load path
> >   Error while executing Scheme code.
> >   (gdb)
> > 
> > This is because the path is not tilde expanded.  In contrast, when
> > sourcing a .py or .gdb script the path is tilde expanded.
> > 
> > This commit fixes this oversight, and allows the above source command
> > to work as expected.
> > 
> > The tilde expansion is done in guile/scm-safe-call.c rather than at a
> > higher level, for example, it might be tempting to add the tilde
> > expansion in cli/cli-cmds.c:source_script_from_stream, however, not
> > every extension language wants to see a tilde expanded file path.  For
> > example, consider Python.  Currently we pass in an unexpanded path and
> > we see this behaviour:
> > 
> >   (gdb) source ~/tmp/xxx.py
> >   Traceback (most recent call last):
> >     File "~/tmp/xxx.py", line 1, in <module>
> >   NameError: name 'undefined' is not defined
> > 
> > If we performed tilde expansion prior to calling the Python sourcer
> > function (gdbpy_source_script), we would instead see:
> > 
> >   (gdb) source ~/tmp/xxx.py
> >   Traceback (most recent call last):
> >     File "/home/andrew/tmp/xxx.py", line 1, in <module>
> >       undefined
> >   NameError: name 'undefined' is not defined
> > 
> > Notice the path expansion in the second line of the error message, I
> > believe this is a change for the worse.
> 
> I really don't have a strong opinion, but I don't think this would be a
> problematic change.  The tilde-expansion thing is really a shell thing
> (and we replicate it in GDB).  But once the paths are processed by
> programs such as Python, the tilde usually doesn't have a special
> meaning.
> 
> If I do:
> 
>     $ python3 ~/foo.py
> 
> I get:
> 
>     Traceback (most recent call last):
>       File "/home/simark/foo.py", line 1, in <module>
> 	blah
>     NameError: name 'blah' is not defined
> 
> Because the shell did the tilde-expansion before passing the file name
> to Python.  If I do:
> 
>     $ python3 '~/foo.py'
> 
> then I get:
> 
>     python3: can't open file '/home/simark/build/binutils-gdb-one-target/gdb/~/foo.py': [Errno 2] No such file or directory
> 
> Python shows the tilde version of the path in our error messages just
> becore we tell it that's the name of the file, Python doesn't use that
> name otherwise (I think).  We could tell it the filename is "potato",
> and it would say that the error is in file "potato".
> 
> So in a sense, passing the tilde-expanded version would make it look
> more like the "regular" Python interpreter.
> 
> But like I said, I don't have a strong opinion: in the end this is shown
> to the user, they will understand it either way.
> 
> Otherwise, the patch LGTM.

I think you convinced me.

How's the patch below?

Thanks,
Andrew

---

commit 25d175146367939dca8b45a155fd85fb0cc28092
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Wed May 5 16:50:17 2021 +0100

    gdb/guile: perform tilde expansion when sourcing guile scripts
    
    Before this patch:
    
      (gdb) source ~/script.scm
      ERROR: In procedure apply-smob/1:
      ERROR: In procedure primitive-load-path: Unable to find file "~/script.scm" in load path
      Error while executing Scheme code.
      (gdb)
    
    This is because the path is not tilde expanded.  In contrast, when
    sourcing a .py or .gdb script the path is tilde expanded.
    
    This commit fixes this oversight, and allows the above source command
    to work as expected.
    
    The tilde expansion is done in the generic GDB code before we call the
    sourcer function for any particular extension language.
    
    gdb/ChangeLog:
    
            * cli/cli-cmds.c: Add 'gdbsupport/gdb_tilde_expand.h'
            include.
            (source_script_with_search): Perform tilde expansion.
    
    gdb/testsuite/ChangeLog:
    
            * gdb.guile/guile.exp: Add an extra test.

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 68ef92eca4c..c641b9db13d 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -54,6 +54,7 @@
 
 #include "extension.h"
 #include "gdbsupport/pathstuff.h"
+#include "gdbsupport/gdb_tilde_expand.h"
 
 #ifdef TUI
 #include "tui/tui.h"	/* For tui_active et.al.  */
@@ -737,8 +738,10 @@ source_script_with_search (const char *file, int from_tty, int search_path)
      anyway so that error messages show the actual file used.  But only do
      this if we (may have) used search_path, as printing the full path in
      errors for the non-search case can be more noise than signal.  */
+  gdb::unique_xmalloc_ptr<char> file_to_open = gdb_tilde_expand_up (file);
   source_script_from_stream (opened->stream.get (), file,
-			     search_path ? opened->full_path.get () : file);
+			     search_path ? opened->full_path.get ()
+			     : file_to_open.get ());
 }
 
 /* Wrapper around source_script_with_search to export it to main.c
diff --git a/gdb/testsuite/gdb.guile/guile.exp b/gdb/testsuite/gdb.guile/guile.exp
index 6e464cc0e77..0fb82284f46 100644
--- a/gdb/testsuite/gdb.guile/guile.exp
+++ b/gdb/testsuite/gdb.guile/guile.exp
@@ -82,3 +82,10 @@ gdb_test_no_output "guile (define a (execute \"help\" #:to-string #t))" \
 
 gdb_test "guile (print a)" "= .*aliases -- User-defined aliases of other commands.*" \
     "verify help to uiout"
+
+# Verify that we can source a guile script using ~ for the HOME directory.
+save_vars { env(HOME) } {
+    set env(HOME) $srcdir/$subdir
+    clean_restart
+    gdb_test "source ~/source2.scm" "yes"
+}

Re: [PATCHv2 1/2] gdb/guile: perform tilde expansion when sourcing guile scripts

[Simon Marchi]

On 2021-05-07 6:29 a.m., Andrew Burgess wrote:
> @@ -737,8 +738,10 @@ source_script_with_search (const char *file, int from_tty, int search_path)
>       anyway so that error messages show the actual file used.  But only do
>       this if we (may have) used search_path, as printing the full path in
>       errors for the non-search case can be more noise than signal.  */
> +  gdb::unique_xmalloc_ptr<char> file_to_open = gdb_tilde_expand_up (file);
>    source_script_from_stream (opened->stream.get (), file,
> -			     search_path ? opened->full_path.get () : file);
> +			     search_path ? opened->full_path.get ()
> +			     : file_to_open.get ());

A minor detail, but the gdb_tilde_expand_up call is only necessary when
search_path is false.  So if there's a easy way to avoid it otherwise,
it would be nice.  But other than that, LGTM.

Simon

Re: [PATCHv2 1/2] gdb/guile: perform tilde expansion when sourcing guile scripts

[Andrew Burgess]

* Simon Marchi <simon.marchi@polymtl.ca> [2021-05-07 10:55:38 -0400]:

> On 2021-05-07 6:29 a.m., Andrew Burgess wrote:
> > @@ -737,8 +738,10 @@ source_script_with_search (const char *file, int from_tty, int search_path)
> >       anyway so that error messages show the actual file used.  But only do
> >       this if we (may have) used search_path, as printing the full path in
> >       errors for the non-search case can be more noise than signal.  */
> > +  gdb::unique_xmalloc_ptr<char> file_to_open = gdb_tilde_expand_up (file);
> >    source_script_from_stream (opened->stream.get (), file,
> > -			     search_path ? opened->full_path.get () : file);
> > +			     search_path ? opened->full_path.get ()
> > +			     : file_to_open.get ());
> 
> A minor detail, but the gdb_tilde_expand_up call is only necessary when
> search_path is false.  So if there's a easy way to avoid it otherwise,
> it would be nice.  But other than that, LGTM.

Thanks.  Below is what I pushed.

---

commit 1845e254645efbc02248345ccdb557d265dd8ae1
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Wed May 5 16:50:17 2021 +0100

    gdb/guile: perform tilde expansion when sourcing guile scripts
    
    Before this patch:
    
      (gdb) source ~/script.scm
      ERROR: In procedure apply-smob/1:
      ERROR: In procedure primitive-load-path: Unable to find file "~/script.scm" in load path
      Error while executing Scheme code.
      (gdb)
    
    This is because the path is not tilde expanded.  In contrast, when
    sourcing a .py or .gdb script the path is tilde expanded.
    
    This commit fixes this oversight, and allows the above source command
    to work as expected.
    
    The tilde expansion is done in the generic GDB code before we call the
    sourcer function for any particular extension language.
    
    gdb/ChangeLog:
    
            * cli/cli-cmds.c: Add 'gdbsupport/gdb_tilde_expand.h'
            include.
            (source_script_with_search): Perform tilde expansion.
    
    gdb/testsuite/ChangeLog:
    
            * gdb.guile/guile.exp: Add an extra test.

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 68ef92eca4c..62948f5fc7b 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -54,6 +54,7 @@
 
 #include "extension.h"
 #include "gdbsupport/pathstuff.h"
+#include "gdbsupport/gdb_tilde_expand.h"
 
 #ifdef TUI
 #include "tui/tui.h"	/* For tui_active et.al.  */
@@ -737,8 +738,16 @@ source_script_with_search (const char *file, int from_tty, int search_path)
      anyway so that error messages show the actual file used.  But only do
      this if we (may have) used search_path, as printing the full path in
      errors for the non-search case can be more noise than signal.  */
-  source_script_from_stream (opened->stream.get (), file,
-			     search_path ? opened->full_path.get () : file);
+  const char *file_to_open;
+  gdb::unique_xmalloc_ptr<char> tilde_expanded_file;
+  if (search_path)
+    file_to_open = opened->full_path.get ();
+  else
+    {
+      tilde_expanded_file = gdb_tilde_expand_up (file);
+      file_to_open = tilde_expanded_file.get ();
+    }
+  source_script_from_stream (opened->stream.get (), file, file_to_open);
 }
 
 /* Wrapper around source_script_with_search to export it to main.c
diff --git a/gdb/testsuite/gdb.guile/guile.exp b/gdb/testsuite/gdb.guile/guile.exp
index 6e464cc0e77..0fb82284f46 100644
--- a/gdb/testsuite/gdb.guile/guile.exp
+++ b/gdb/testsuite/gdb.guile/guile.exp
@@ -82,3 +82,10 @@ gdb_test_no_output "guile (define a (execute \"help\" #:to-string #t))" \
 
 gdb_test "guile (print a)" "= .*aliases -- User-defined aliases of other commands.*" \
     "verify help to uiout"
+
+# Verify that we can source a guile script using ~ for the HOME directory.
+save_vars { env(HOME) } {
+    set env(HOME) $srcdir/$subdir
+    clean_restart
+    gdb_test "source ~/source2.scm" "yes"
+}

[PATCHv2 2/2] gdb/guile: Have gdbscm_safe_source_script return a unique_ptr

[Andrew Burgess]

Change gdbscm_safe_source_script to return a
gdb::unique_xmalloc_ptr<char> instead of a raw char*.  Update the
users of this function.

There should be no user visible change after this commit.

gdb/ChangeLog:

	* guile/guile-internal.h (gdbscm_safe_source_script): Change
	function return type.
	* guile/guile.c (gdbscm_source_script): Update to handle change in
	gdbscm_safe_source_script.
	* guile/scm-objfile.c (gdbscm_source_objfile_script): Likewise.
	* guile/scm-safe-call.c (gdbscm_safe_source_script): Change return
	type.
---
 gdb/ChangeLog              | 10 ++++++++++
 gdb/guile/guile-internal.h |  3 ++-
 gdb/guile/guile.c          |  7 ++-----
 gdb/guile/scm-objfile.c    |  9 ++-------
 gdb/guile/scm-safe-call.c  |  4 ++--
 5 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/gdb/guile/guile-internal.h b/gdb/guile/guile-internal.h
index c48547a41cd..73fa3803dfd 100644
--- a/gdb/guile/guile-internal.h
+++ b/gdb/guile/guile-internal.h
@@ -408,7 +408,8 @@ extern SCM gdbscm_unsafe_call_1 (SCM proc, SCM arg0);
 extern gdb::unique_xmalloc_ptr<char> gdbscm_safe_eval_string
   (const char *string, int display_result);
 
-extern char *gdbscm_safe_source_script (const char *filename);
+extern gdb::unique_xmalloc_ptr<char> gdbscm_safe_source_script
+  (const char *filename);
 
 extern void gdbscm_enter_repl (void);
 \f
diff --git a/gdb/guile/guile.c b/gdb/guile/guile.c
index c6959f5b713..71d61f375b3 100644
--- a/gdb/guile/guile.c
+++ b/gdb/guile/guile.c
@@ -270,13 +270,10 @@ static void
 gdbscm_source_script (const struct extension_language_defn *extlang,
 		      FILE *file, const char *filename)
 {
-  char *msg = gdbscm_safe_source_script (filename);
+  gdb::unique_xmalloc_ptr<char> msg = gdbscm_safe_source_script (filename);
 
   if (msg != NULL)
-    {
-      fprintf_filtered (gdb_stderr, "%s\n", msg);
-      xfree (msg);
-    }
+    fprintf_filtered (gdb_stderr, "%s\n", msg.get ());
 }
 \f
 /* (execute string [#:from-tty boolean] [#:to-string boolean])
diff --git a/gdb/guile/scm-objfile.c b/gdb/guile/scm-objfile.c
index 30e63f374e7..cc8bbe63a70 100644
--- a/gdb/guile/scm-objfile.c
+++ b/gdb/guile/scm-objfile.c
@@ -310,16 +310,11 @@ gdbscm_source_objfile_script (const struct extension_language_defn *extlang,
 			      struct objfile *objfile, FILE *file,
 			      const char *filename)
 {
-  char *msg;
-
   ofscm_current_objfile = objfile;
 
-  msg = gdbscm_safe_source_script (filename);
+  gdb::unique_xmalloc_ptr<char> msg = gdbscm_safe_source_script (filename);
   if (msg != NULL)
-    {
-      fprintf_filtered (gdb_stderr, "%s", msg);
-      xfree (msg);
-    }
+    fprintf_filtered (gdb_stderr, "%s", msg.get ());
 
   ofscm_current_objfile = NULL;
 }
diff --git a/gdb/guile/scm-safe-call.c b/gdb/guile/scm-safe-call.c
index 5bea970239b..a475774f921 100644
--- a/gdb/guile/scm-safe-call.c
+++ b/gdb/guile/scm-safe-call.c
@@ -432,7 +432,7 @@ scscm_source_scheme_script (void *data)
    printed according to "set guile print-stack" and the result is an error
    message allocated with malloc, caller must free.  */
 
-char *
+gdb::unique_xmalloc_ptr<char>
 gdbscm_safe_source_script (const char *filename)
 {
   /* scm_c_primitive_load_path only looks in %load-path for files with
@@ -454,7 +454,7 @@ gdbscm_safe_source_script (const char *filename)
 			      (void *) filename);
 
   if (result != NULL)
-    return xstrdup (result);
+    return make_unique_xstrdup (result);
   return NULL;
 }
 \f
-- 
2.25.4

Re: [PATCHv2 2/2] gdb/guile: Have gdbscm_safe_source_script return a unique_ptr

[Simon Marchi]

On 2021-05-05 5:01 p.m., Andrew Burgess wrote:
> diff --git a/gdb/guile/scm-safe-call.c b/gdb/guile/scm-safe-call.c
> index 5bea970239b..a475774f921 100644
> --- a/gdb/guile/scm-safe-call.c
> +++ b/gdb/guile/scm-safe-call.c
> @@ -432,7 +432,7 @@ scscm_source_scheme_script (void *data)
>     printed according to "set guile print-stack" and the result is an error
>     message allocated with malloc, caller must free.  */
>  
> -char *
> +gdb::unique_xmalloc_ptr<char>
>  gdbscm_safe_source_script (const char *filename)
>  {
>    /* scm_c_primitive_load_path only looks in %load-path for files with
> @@ -454,7 +454,7 @@ gdbscm_safe_source_script (const char *filename)
>  			      (void *) filename);
>  
>    if (result != NULL)
> -    return xstrdup (result);
> +    return make_unique_xstrdup (result);
>    return NULL;

Hmm, result comes from gdbscm_with_guile, which states:

   The result if NULL if no exception occurred, otherwise it is a statically
   allocated error message (caller must *not* free).  */

Simon

Re: [PATCHv2 2/2] gdb/guile: Have gdbscm_safe_source_script return a unique_ptr

[Simon Marchi]

On 2021-05-05 10:28 p.m., Simon Marchi via Gdb-patches wrote:
> On 2021-05-05 5:01 p.m., Andrew Burgess wrote:
>> diff --git a/gdb/guile/scm-safe-call.c b/gdb/guile/scm-safe-call.c
>> index 5bea970239b..a475774f921 100644
>> --- a/gdb/guile/scm-safe-call.c
>> +++ b/gdb/guile/scm-safe-call.c
>> @@ -432,7 +432,7 @@ scscm_source_scheme_script (void *data)
>>     printed according to "set guile print-stack" and the result is an error
>>     message allocated with malloc, caller must free.  */
>>  
>> -char *
>> +gdb::unique_xmalloc_ptr<char>
>>  gdbscm_safe_source_script (const char *filename)
>>  {
>>    /* scm_c_primitive_load_path only looks in %load-path for files with
>> @@ -454,7 +454,7 @@ gdbscm_safe_source_script (const char *filename)
>>  			      (void *) filename);
>>  
>>    if (result != NULL)
>> -    return xstrdup (result);
>> +    return make_unique_xstrdup (result);
>>    return NULL;
> 
> Hmm, result comes from gdbscm_with_guile, which states:
> 
>    The result if NULL if no exception occurred, otherwise it is a statically
>    allocated error message (caller must *not* free).  */

Ahh, nevermind.  I noticed now that it's "make_unique_xstrdup".  I first
read it as just creating a unique pointer to wrap result.

LGTM then!

Simon

Re: [PATCHv2 2/2] gdb/guile: Have gdbscm_safe_source_script return a unique_ptr

[Andrew Burgess]

* Simon Marchi <simon.marchi@polymtl.ca> [2021-05-05 22:32:40 -0400]:

> On 2021-05-05 10:28 p.m., Simon Marchi via Gdb-patches wrote:
> > On 2021-05-05 5:01 p.m., Andrew Burgess wrote:
> >> diff --git a/gdb/guile/scm-safe-call.c b/gdb/guile/scm-safe-call.c
> >> index 5bea970239b..a475774f921 100644
> >> --- a/gdb/guile/scm-safe-call.c
> >> +++ b/gdb/guile/scm-safe-call.c
> >> @@ -432,7 +432,7 @@ scscm_source_scheme_script (void *data)
> >>     printed according to "set guile print-stack" and the result is an error
> >>     message allocated with malloc, caller must free.  */
> >>  
> >> -char *
> >> +gdb::unique_xmalloc_ptr<char>
> >>  gdbscm_safe_source_script (const char *filename)
> >>  {
> >>    /* scm_c_primitive_load_path only looks in %load-path for files with
> >> @@ -454,7 +454,7 @@ gdbscm_safe_source_script (const char *filename)
> >>  			      (void *) filename);
> >>  
> >>    if (result != NULL)
> >> -    return xstrdup (result);
> >> +    return make_unique_xstrdup (result);
> >>    return NULL;
> > 
> > Hmm, result comes from gdbscm_with_guile, which states:
> > 
> >    The result if NULL if no exception occurred, otherwise it is a statically
> >    allocated error message (caller must *not* free).  */
> 
> Ahh, nevermind.  I noticed now that it's "make_unique_xstrdup".  I first
> read it as just creating a unique pointer to wrap result.
> 
> LGTM then!

Thanks.  I pushed this patch independently of patch #1.

Andrew