* [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: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-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-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