Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [python] [patch] PR python/13345
@ 2011-10-31 18:11 Phil Muldoon
  2011-11-01 14:07 ` Meador Inge
  2011-11-01 18:55 ` Tom Tromey
  0 siblings, 2 replies; 6+ messages in thread
From: Phil Muldoon @ 2011-10-31 18:11 UTC (permalink / raw)
  To: gdb-patches


This patch fixes a case where the tilde (~) command was being passed to
Python via the "source" command.  Python does not understand what to do
with a tilde, so we have to expand it first.

OK?

Cheers,

Phil

--

2011-10-31  Phil Muldoon  <pmuldoon@redhat.com>

	PR Python/13345

	* python/python.c (python_run_simple_file): Expand tilde in path.

--

Index: python/python.c
===================================================================
RCS file: /cvs/src/src/gdb/python/python.c,v
retrieving revision 1.75
diff -u -r1.75 python.c
--- python/python.c	7 Oct 2011 07:38:30 -0000	1.75
+++ python/python.c	31 Oct 2011 15:42:19 -0000
@@ -30,6 +30,7 @@
 #include "exceptions.h"
 #include "event-loop.h"
 #include "serial.h"
+#include "readline/tilde.h"
 #include "python.h"
 
 #include <ctype.h>
@@ -162,13 +163,22 @@
 static void
 python_run_simple_file (const char *filename)
 {
-  char *filename_copy;
+  char *full_path;
   PyObject *python_file;
   struct cleanup *cleanup;
 
-  filename_copy = xstrdup (filename);
-  cleanup = make_cleanup (xfree, filename_copy);
-  python_file = PyFile_FromString (filename_copy, "r");
+  /* Because we have a string for a filename, and are using Python to
+     open the file, we need to expand any tilde in the path first.  */
+  full_path = tilde_expand (filename);
+  cleanup = make_cleanup (xfree, full_path);
+  python_file = PyFile_FromString (full_path, "r");
+  if (! python_file)
+    {
+      do_cleanups (cleanup);
+      gdbpy_print_stack ();
+      error (_("Error sourcing Python file"));
+    }
+ 
   make_cleanup_py_decref (python_file);
   PyRun_SimpleFile (PyFile_AsFile (python_file), filename);
   do_cleanups (cleanup);


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [python] [patch] PR python/13345
  2011-10-31 18:11 [python] [patch] PR python/13345 Phil Muldoon
@ 2011-11-01 14:07 ` Meador Inge
  2011-11-01 14:15   ` Phil Muldoon
  2011-11-01 17:53   ` Tom Tromey
  2011-11-01 18:55 ` Tom Tromey
  1 sibling, 2 replies; 6+ messages in thread
From: Meador Inge @ 2011-11-01 14:07 UTC (permalink / raw)
  To: pmuldoon; +Cc: gdb-patches

On 10/31/2011 10:51 AM, Phil Muldoon wrote:

> This patch fixes a case where the tilde (~) command was being passed to
> Python via the "source" command.  Python does not understand what to do
> with a tilde, so we have to expand it first.

While I can't give an OK, I did review this and was able to reproduce
the stated problem and the patch fixes it.  So, LGTM.  What about
a test case, though?  You could construct a relative path to a
test directory from '~/'.


-- 
Meador Inge
CodeSourcery / Mentor Embedded
http://www.mentor.com/embedded-software


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [python] [patch] PR python/13345
  2011-11-01 14:07 ` Meador Inge
@ 2011-11-01 14:15   ` Phil Muldoon
  2011-11-01 18:06     ` Meador Inge
  2011-11-01 17:53   ` Tom Tromey
  1 sibling, 1 reply; 6+ messages in thread
From: Phil Muldoon @ 2011-11-01 14:15 UTC (permalink / raw)
  To: Meador Inge; +Cc: gdb-patches

Meador Inge <meadori@codesourcery.com> writes:

> On 10/31/2011 10:51 AM, Phil Muldoon wrote:
>
>> This patch fixes a case where the tilde (~) command was being passed to
>> Python via the "source" command.  Python does not understand what to do
>> with a tilde, so we have to expand it first.
>
> While I can't give an OK, I did review this and was able to reproduce
> the stated problem and the patch fixes it.  So, LGTM.  What about
> a test case, though?  You could construct a relative path to a
> test directory from '~/'.

The only addition in the patch was tilde_expand, and an additional error
check.  tilde_expand is a readline function.  So we would be testing
that, more or less.  I do normally write regression tests, but I felt
for this one it was not necessary as the patch is somewhat trivial.
Plus I am not sure how constructing a path with a ~ in it would work on
mingw builds?  If so, and we really do want one, I can attempt to write
one.

Cheers

Phil


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [python] [patch] PR python/13345
  2011-11-01 14:07 ` Meador Inge
  2011-11-01 14:15   ` Phil Muldoon
@ 2011-11-01 17:53   ` Tom Tromey
  1 sibling, 0 replies; 6+ messages in thread
From: Tom Tromey @ 2011-11-01 17:53 UTC (permalink / raw)
  To: Meador Inge; +Cc: pmuldoon, gdb-patches

>>>>> "Meador" == Meador Inge <meadori@codesourcery.com> writes:

Meador> On 10/31/2011 10:51 AM, Phil Muldoon wrote:
>> This patch fixes a case where the tilde (~) command was being passed to
>> Python via the "source" command.  Python does not understand what to do
>> with a tilde, so we have to expand it first.

Meador> While I can't give an OK, I did review this and was able to reproduce
Meador> the stated problem and the patch fixes it.  So, LGTM.  What about
Meador> a test case, though?  You could construct a relative path to a
Meador> test directory from '~/'.

I think in this case it is ok not to have a test.

But if you want to write one it would be fine by me.
It would have to be xfail'd if not under $HOME.

Tom


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [python] [patch] PR python/13345
  2011-11-01 14:15   ` Phil Muldoon
@ 2011-11-01 18:06     ` Meador Inge
  0 siblings, 0 replies; 6+ messages in thread
From: Meador Inge @ 2011-11-01 18:06 UTC (permalink / raw)
  To: pmuldoon; +Cc: gdb-patches

On 11/01/2011 09:14 AM, Phil Muldoon wrote:
> Meador Inge <meadori@codesourcery.com> writes:
> 
>> On 10/31/2011 10:51 AM, Phil Muldoon wrote:
>>
>>> This patch fixes a case where the tilde (~) command was being passed to
>>> Python via the "source" command.  Python does not understand what to do
>>> with a tilde, so we have to expand it first.
>>
>> While I can't give an OK, I did review this and was able to reproduce
>> the stated problem and the patch fixes it.  So, LGTM.  What about
>> a test case, though?  You could construct a relative path to a
>> test directory from '~/'.
> 
> The only addition in the patch was tilde_expand, and an additional error
> check.  tilde_expand is a readline function.  So we would be testing
> that, more or less.  I do normally write regression tests, but I felt
> for this one it was not necessary as the patch is somewhat trivial.
> Plus I am not sure how constructing a path with a ~ in it would work on
> mingw builds?  If so, and we really do want one, I can attempt to write
> one.

That seems reasonable.  I agree the test case can be skipped.  Just figured I
would ask ...

-- 
Meador Inge
CodeSourcery / Mentor Embedded


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [python] [patch] PR python/13345
  2011-10-31 18:11 [python] [patch] PR python/13345 Phil Muldoon
  2011-11-01 14:07 ` Meador Inge
@ 2011-11-01 18:55 ` Tom Tromey
  1 sibling, 0 replies; 6+ messages in thread
From: Tom Tromey @ 2011-11-01 18:55 UTC (permalink / raw)
  To: pmuldoon; +Cc: gdb-patches

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> 2011-10-31  Phil Muldoon  <pmuldoon@redhat.com>
Phil> 	PR Python/13345
Phil> 	* python/python.c (python_run_simple_file): Expand tilde in path.

Phil> +  python_file = PyFile_FromString (full_path, "r");
Phil> +  if (! python_file)
Phil> +    {
Phil> +      do_cleanups (cleanup);
Phil> +      gdbpy_print_stack ();
Phil> +      error (_("Error sourcing Python file"));

I think the error should say "Error while opening file: %s"
and have the file name.

Ok with that change.

Tom


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-11-01 18:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-31 18:11 [python] [patch] PR python/13345 Phil Muldoon
2011-11-01 14:07 ` Meador Inge
2011-11-01 14:15   ` Phil Muldoon
2011-11-01 18:06     ` Meador Inge
2011-11-01 17:53   ` Tom Tromey
2011-11-01 18:55 ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox