This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Eliminate UNSUPPORTED_ERROR.
- From: Pedro Alves <palves at redhat dot com>
- To: Doug Evans <xdje42 at gmail dot com>
- Cc: Hui Zhu <hui_zhu at mentor dot com>, gdb-patches ml <gdb-patches at sourceware dot org>
- Date: Thu, 12 Dec 2013 10:22:53 +0000
- Subject: Re: [PATCH] Eliminate UNSUPPORTED_ERROR.
- Authentication-results: sourceware.org; auth=none
- References: <5265022F dot 8060203 at mentor dot com> <52654A2C dot 9010202 at redhat dot com> <529707C7 dot 4040504 at mentor dot com> <5298AE7C dot 6020607 at redhat dot com> <529C80D2 dot 2080608 at mentor dot com> <529C9B42 dot 20600 at redhat dot com> <529D62F7 dot 80701 at mentor dot com> <52A22582 dot 8040509 at redhat dot com> <52A40015 dot 207 at mentor dot com> <52A61E86 dot 3020005 at redhat dot com> <m31u1lvimb dot fsf at sspiff dot org> <52A750AA dot 1080807 at redhat dot com> <52A75A05 dot 60006 at redhat dot com> <CAP9bCMRyMQESGWQFSsB-+OJ=eSgtcnNcC6J4hDxq5n6kSY=c6Q at mail dot gmail dot com> <52A8BA3D dot 9020403 at redhat dot com> <CAP9bCMTk4RzQnte3Nq57TpzwdkxWA0hXOPaS9u0ST9oP+k2AxA at mail dot gmail dot com>
On 12/12/2013 04:22 AM, Doug Evans wrote:
> On Wed, Dec 11, 2013 at 11:17 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 12/11/2013 04:33 PM, Doug Evans wrote:
>>>>> - /* Should we fallback to ye olde GDB script mode? */
>>>>> - if (script_ext_mode == script_ext_soft
>>>>> - && e.reason == RETURN_ERROR && e.error == UNSUPPORTED_ERROR)
>>>>> - {
>>>>> - fseek (stream, 0, SEEK_SET);
>>>>> - script_from_file (stream, (char*) file);
>>>>> - }
>>>>> - else
>>>>> - {
>>>>> - /* Nope, just punt. */
>>>>> - throw_exception (e);
>>>>> - }
>>>>> + /* Fallback to GDB script mode. */
>>>>> + fseek (stream, 0, SEEK_SET);
>>>>> + script_from_file (stream, (char*) file);
>>> Remove the fseek and cast.
>>
>> Hmm, indeed the fseek doesn't look necessary. Will do. However,
>> that makes me wonder whether I'm missing something, as it doesn't
>> look necessary before my patch either. Was it ever really
>> needed?
>
> I think I added it to be extra conservative in case
> source_python_script happened to read from the file.
> [I know it normally wouldn't read and then throw an UNSUPPORTED_ERROR,
> but I'm guessing I didn't want this code to assume that.]
>
> Now that we're not even calling source_python_script the need is gone.
>
> I see you added it (and the unnecessary cast too ;-) ) in:
>> https://www.sourceware.org/ml/gdb-patches/2010-04/msg00110.html
>> but I can't tell why.
>
> script_from_file was changed to take a const char *arg the prior week.
> Presumably I just didn't update that part of the patch.
>
Alright, pushed, as below.
--------
Eliminate UNSUPPORTED_ERROR.
I have a case that could use an exception for "unsupported feature".
I found UNSUPPORTED_ERROR, but looking deeper, I think as is, reusing
it for other things would be fragile. E.g., if the Python script
sourced by source_script_from_stream triggers any other missing
functionality that would result in UNSUPPORTED_ERROR being propagated
out to source_script_from_stream, that would confuse the error for
Python not being built into GDB.
This patch thus redoes things a little. Instead of using an exception
for the "No Python" scenario, check whether Python is configured in
before actually trying to source the file. It adds a new function
instead of using #ifdef HAVE_PYTHON directly, as that is better at
avoiding bitrot, as both Python and !Python paths are visible to the
compiler this way.
Tested on Fedora 17, with and without Python.
gdb/
2013-12-12 Pedro Alves <palves@redhat.com>
* cli/cli-cmds.c (source_script_from_stream) Use have_python
instead of catching UNSUPPORTED_ERROR.
* exceptions.h (UNSUPPORTED_ERROR): Delete.
* python/python.c (source_python_script) [!HAVE_PYTHON]: Internal
error if called.
* python/python.h (have_python): New static inline function.
---
gdb/ChangeLog | 9 +++++++++
gdb/cli/cli-cmds.c | 26 +++++++-------------------
gdb/exceptions.h | 3 ---
gdb/python/python.c | 5 +++--
gdb/python/python.h | 13 +++++++++++++
5 files changed, 32 insertions(+), 24 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 40368b5..637a462 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2013-12-12 Pedro Alves <palves@redhat.com>
+
+ * cli/cli-cmds.c (source_script_from_stream) Use have_python
+ instead of catching UNSUPPORTED_ERROR.
+ * exceptions.h (UNSUPPORTED_ERROR): Delete.
+ * python/python.c (source_python_script) [!HAVE_PYTHON]: Internal
+ error if called.
+ * python/python.h (have_python): New static inline function.
+
2013-12-11 Doug Evans <dje@google.com>
* dwarf2read.c (lookup_dwo_cutu): Include name of dwp file in
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 52a6bc9..73e03cf 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -525,27 +525,15 @@ 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"))
{
- volatile struct gdb_exception e;
-
- TRY_CATCH (e, RETURN_MASK_ERROR)
+ if (have_python ())
+ source_python_script (stream, file);
+ else if (script_ext_mode == script_ext_soft)
{
- source_python_script (stream, file);
- }
- if (e.reason < 0)
- {
- /* Should we fallback to ye olde GDB script mode? */
- if (script_ext_mode == script_ext_soft
- && e.reason == RETURN_ERROR && e.error == UNSUPPORTED_ERROR)
- {
- fseek (stream, 0, SEEK_SET);
- script_from_file (stream, (char*) file);
- }
- else
- {
- /* Nope, just punt. */
- throw_exception (e);
- }
+ /* Fallback to GDB script mode. */
+ script_from_file (stream, file);
}
+ else
+ error (_("Python scripting is not supported in this copy of GDB."));
}
else
script_from_file (stream, file);
diff --git a/gdb/exceptions.h b/gdb/exceptions.h
index 705f1a1..2cb8242 100644
--- a/gdb/exceptions.h
+++ b/gdb/exceptions.h
@@ -79,9 +79,6 @@ enum errors {
/* Error accessing memory. */
MEMORY_ERROR,
- /* Feature is not supported in this copy of GDB. */
- UNSUPPORTED_ERROR,
-
/* Value not available. E.g., a register was not collected in a
traceframe. */
NOT_AVAILABLE_ERROR,
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 1873936..55bb6cf 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1390,8 +1390,9 @@ eval_python_from_control_command (struct command_line *cmd)
void
source_python_script (FILE *file, const char *filename)
{
- throw_error (UNSUPPORTED_ERROR,
- _("Python scripting is not supported in this copy of GDB."));
+ internal_error (__FILE__, __LINE__,
+ _("source_python_script called when Python scripting is "
+ "not supported."));
}
int
diff --git a/gdb/python/python.h b/gdb/python/python.h
index c07b2aa..abbb581 100644
--- a/gdb/python/python.h
+++ b/gdb/python/python.h
@@ -89,6 +89,19 @@ typedef enum py_frame_args
CLI_ALL_VALUES
} py_frame_args;
+/* Returns true if Python support is built into GDB, false
+ otherwise. */
+
+static inline int
+have_python (void)
+{
+#ifdef HAVE_PYTHON
+ return 1;
+#else
+ return 0;
+#endif
+}
+
extern void finish_python_initialization (void);
void eval_python_from_control_command (struct command_line *);
--
1.7.11.7