Fix build with Python 3.4 (PR python/16784)
Commit Message
The type of the function pointer PyOS_ReadlineFunctionPointer (part of the
Python C API), which we use, slightly changed starting with Python 3.4. The
signature when from
PyAPI_DATA(char) *(*PyOS_ReadlineFunctionPointer)(FILE *, FILE *, char *);
to
PyAPI_DATA(char) *(*PyOS_ReadlineFunctionPointer)(FILE *, FILE *, const char *);
The parameter that changed is the prompt text.
This commits adjusts gdb accordingly by making the prompt_arg parameter
const, as well as the fallouts. Because of that, I needed to rework a
bit how annotations are added to the prompt. If they are enabled, a copy
of the prompt is made (as before) but then the prompt_arg argument is
overwritten to point to the annotated prompt. Otherwise, no copy is
done and the original prompt_arg is passed to called functions.
I changed the signature of deprecated_readline_hook. I would've changed any
user of it, but it seems like nothing is using it,
Built-tested with Python 2.7.8, 3.3.6 and 3.4.2.
I ran the tests in gdb.base and gdb.python before and after with Python
2.7.8 and there was no difference in the results.
gdb/ChangeLog:
* defs.h (gdb_readline): Constify argument.
(gdb_readline_wrapper): Same.
(command_line_input): Same.
(deprecated_readline_hook): Same.
* top.c (deprecated_readline_hook): Same.
(gdb_readline): Same.
(gdb_readline_wrapper): Same.
(command_line_input): Constify argument. Pass prompt_arg to
called functions instead of local_prompt, overwriting prompt_arg
if necessary.
* event-top.h (display_gdb_prompt): Constify argument.
* event-top.c (display_gdb_prompt): Same.
* python/py-gdb-readline.c (gdbpy_readline_wrapper): Constify
argument if building with Python 3.4 and up.
---
gdb/defs.h | 8 ++++----
gdb/event-top.c | 2 +-
gdb/event-top.h | 2 +-
gdb/python/py-gdb-readline.c | 5 ++++-
gdb/top.c | 20 +++++++++++---------
5 files changed, 21 insertions(+), 16 deletions(-)
Comments
On 11/26/2014 01:47 PM, Simon Marchi wrote:
> The type of the function pointer PyOS_ReadlineFunctionPointer (part of the
> Python C API), which we use, slightly changed starting with Python 3.4. The
> signature when from
"went from"
> I changed the signature of deprecated_readline_hook. I would've changed any
> user of it, but it seems like nothing is using it,
You'd probably find it in the insight/gdbtk code.
> --- a/gdb/python/py-gdb-readline.c
> +++ b/gdb/python/py-gdb-readline.c
> @@ -26,10 +26,13 @@
> cases, sys_stdin and sys_stdout are always stdin and stdout
> respectively, as far as I can tell; they are ignored and
> command_line_input is used instead. */
> -
> static char *
Don't lose the empty line.
> gdbpy_readline_wrapper (FILE *sys_stdin, FILE *sys_stdout,
> +#if PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION >= 4
> + const char *prompt)
> +#else
> char *prompt)
> +#endif
>
> @@ -938,6 +938,8 @@ command_line_input (char *prompt_arg, int repeat, char *annotation_suffix)
> strcat (local_prompt, "\n\032\032");
> strcat (local_prompt, annotation_suffix);
> strcat (local_prompt, "\n");
> +
> + prompt_arg = local_prompt;
I think it'd be even clearer if we avoided changing
the variable that is called "arg". How about we add:
const char *prompt = prompt_arg;
at the top, and then use "prompt" throughout, where
you're using "prompt_arg"?
The 'local_prompt' variable could move to the "annotation_level > 1"
block too, for clarity.
Thanks,
Pedro Alves
On 2014-12-04 01:14 PM, Pedro Alves wrote:
> On 11/26/2014 01:47 PM, Simon Marchi wrote:
>> The type of the function pointer PyOS_ReadlineFunctionPointer (part of the
>> Python C API), which we use, slightly changed starting with Python 3.4. The
>> signature when from
>
> "went from"
Done.
>> I changed the signature of deprecated_readline_hook. I would've changed any
>> user of it, but it seems like nothing is using it,
>
> You'd probably find it in the insight/gdbtk code.
Is this something that I need to update, or insight is not maintained anymore?
>> --- a/gdb/python/py-gdb-readline.c
>> +++ b/gdb/python/py-gdb-readline.c
>> @@ -26,10 +26,13 @@
>> cases, sys_stdin and sys_stdout are always stdin and stdout
>> respectively, as far as I can tell; they are ignored and
>> command_line_input is used instead. */
>> -
>> static char *
>
> Don't lose the empty line.
Done.
>> gdbpy_readline_wrapper (FILE *sys_stdin, FILE *sys_stdout,
>> +#if PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION >= 4
>> + const char *prompt)
>> +#else
>> char *prompt)
>> +#endif
>>
>
>> @@ -938,6 +938,8 @@ command_line_input (char *prompt_arg, int repeat, char *annotation_suffix)
>> strcat (local_prompt, "\n\032\032");
>> strcat (local_prompt, annotation_suffix);
>> strcat (local_prompt, "\n");
>> +
>> + prompt_arg = local_prompt;
>
> I think it'd be even clearer if we avoided changing
> the variable that is called "arg". How about we add:
>
> const char *prompt = prompt_arg;
>
> at the top, and then use "prompt" throughout, where
> you're using "prompt_arg"?
How about we just don't call it *_arg?
I don't see the point in having both a prompt and a prompt_arg. If we needed to use both at the same time, of course, but it's not the case.
> The 'local_prompt' variable could move to the "annotation_level > 1"
> block too, for clarity.
Yes, for sure.
Thanks,
Simon
On Friday, December 05 2014, Simon Marchi wrote:
>>> I changed the signature of deprecated_readline_hook. I would've changed any
>>> user of it, but it seems like nothing is using it,
>>
>> You'd probably find it in the insight/gdbtk code.
>
> Is this something that I need to update, or insight is not maintained anymore?
Insight is maintained by Keith Seitz (Cc'ed in this message). My first
reaction is that it would be good if you updated it.
Thanks,
On 12/05/2014 03:00 PM, Sergio Durigan Junior wrote:
> On Friday, December 05 2014, Simon Marchi wrote:
>
>>>> I changed the signature of deprecated_readline_hook. I would've changed any
>>>> user of it, but it seems like nothing is using it,
>>>
>>> You'd probably find it in the insight/gdbtk code.
>>
>> Is this something that I need to update, or insight is not maintained anymore?
>
> Insight is maintained by Keith Seitz (Cc'ed in this message). My first
> reaction is that it would be good if you updated it.
As reported elsewhere, Insight is a bit in limbo right now as I try to
figure out how to best deal with gdb moving to git (and start my own
migration).
There is a temporary repository set up by the Fedora packaging
maintainer, Patrick Monnerat. I believe this was also referenced earlier.
IMO, I would take that as the official repository for the time being.
You can update it, or I can. It's not a huge deal; just let me know.
Keith
On 2014-12-05 06:07 PM, Keith Seitz wrote:
> On 12/05/2014 03:00 PM, Sergio Durigan Junior wrote:
>> On Friday, December 05 2014, Simon Marchi wrote:
>>
>>>>> I changed the signature of deprecated_readline_hook. I would've changed any
>>>>> user of it, but it seems like nothing is using it,
>>>>
>>>> You'd probably find it in the insight/gdbtk code.
>>>
>>> Is this something that I need to update, or insight is not maintained anymore?
>>
>> Insight is maintained by Keith Seitz (Cc'ed in this message). My first
>> reaction is that it would be good if you updated it.
>
> As reported elsewhere, Insight is a bit in limbo right now as I try to
> figure out how to best deal with gdb moving to git (and start my own
> migration).
>
> There is a temporary repository set up by the Fedora packaging
> maintainer, Patrick Monnerat. I believe this was also referenced earlier.
>
> IMO, I would take that as the official repository for the time being.
>
> You can update it, or I can. It's not a huge deal; just let me know.
(CCing Patrick Monnerat, see above for context)
Here is the corresponding change in Insight (as expected very simple):
https://github.com/simark/insight/commit/47810ab5fcbd528dc6b4422c93b5ecaccf2e4fe8
Since binutils-gdb is used as a submodule in the insight repo, this change should
only be merged when the submodule is updated and includes the patch that changes the
signature.
Simon
Simon Marchi wrote:
>>>>>> I changed the signature of deprecated_readline_hook. I would've
>>>>>> changed any user of it, but it seems like nothing is using it,
>>>>>
>>>>> You'd probably find it in the insight/gdbtk code.
>>>>
>>>> Is this something that I need to update, or insight is not
maintained anymore?
>>>
>>> Insight is maintained by Keith Seitz (Cc'ed in this message). My
>>> first reaction is that it would be good if you updated it.
>>
>> As reported elsewhere, Insight is a bit in limbo right now as I try
to
>> figure out how to best deal with gdb moving to git (and start my own
>> migration).
>>
>> There is a temporary repository set up by the Fedora packaging
>> maintainer, Patrick Monnerat. I believe this was also referenced
earlier.
>>
>> IMO, I would take that as the official repository for the time being.
>>
>> You can update it, or I can. It's not a huge deal; just let me know.
> (CCing Patrick Monnerat, see above for context)
> Here is the corresponding change in Insight (as expected very simple):
>
https://github.com/simark/insight/commit/47810ab5fcbd528dc6b4422c93b5eca
ccf2e4fe8
> Since binutils-gdb is used as a submodule in the insight repo, this
change should only be merged when the submodule is updated and includes
the patch that changes the signature.
Thanks for the info and the patch.
However I cannot find yet any commit "xyz" in the official repo where
the deprecated_readline_hook signature is changed.
I regularly check this kind of troubles against the master@official
binutils-gdb repository (~twice a week: the last time was this morning)
and I will react as soon as the compilation fails.
Regards,
Patrick
Simon Marchi wrote:
>>>>>> I changed the signature of deprecated_readline_hook.
> Here is the corresponding change in Insight (as expected very simple):
>
https://github.com/simark/insight/commit/47810ab5fcbd528dc6b4422c93b5eca
ccf2e4fe8
> Since binutils-gdb is used as a submodule in the insight repo, this
change should only be merged when the submodule is updated and includes
the patch that changes the signature.
Pushed. Many thanks.
Patrick
@@ -259,11 +259,11 @@ extern void print_transfer_performance (struct ui_file *stream,
typedef void initialize_file_ftype (void);
-extern char *gdb_readline (char *);
+extern char *gdb_readline (const char *);
-extern char *gdb_readline_wrapper (char *);
+extern char *gdb_readline_wrapper (const char *);
-extern char *command_line_input (char *, int, char *);
+extern char *command_line_input (const char *, int, char *);
extern void print_prompt (void);
@@ -636,7 +636,7 @@ extern void (*deprecated_warning_hook) (const char *, va_list)
extern void (*deprecated_interactive_hook) (void);
extern void (*deprecated_readline_begin_hook) (char *, ...)
ATTRIBUTE_FPTR_PRINTF_1;
-extern char *(*deprecated_readline_hook) (char *);
+extern char *(*deprecated_readline_hook) (const char *);
extern void (*deprecated_readline_end_hook) (void);
extern void (*deprecated_register_changed_hook) (int regno);
extern void (*deprecated_context_hook) (int);
@@ -288,7 +288,7 @@ gdb_rl_callback_handler_reinstall (void)
3. On prompting for pagination. */
void
-display_gdb_prompt (char *new_prompt)
+display_gdb_prompt (const char *new_prompt)
{
char *actual_gdb_prompt = NULL;
struct cleanup *old_chain;
@@ -27,7 +27,7 @@ struct cmd_list_element;
/* Exported functions from event-top.c.
FIXME: these should really go into top.h. */
-extern void display_gdb_prompt (char *new_prompt);
+extern void display_gdb_prompt (const char *new_prompt);
void gdb_setup_readline (void);
void gdb_disable_readline (void);
extern void async_init_signals (void);
@@ -26,10 +26,13 @@
cases, sys_stdin and sys_stdout are always stdin and stdout
respectively, as far as I can tell; they are ignored and
command_line_input is used instead. */
-
static char *
gdbpy_readline_wrapper (FILE *sys_stdin, FILE *sys_stdout,
+#if PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION >= 4
+ const char *prompt)
+#else
char *prompt)
+#endif
{
int n;
char *p = NULL, *q;
@@ -215,7 +215,7 @@ void (*deprecated_warning_hook) (const char *, va_list);
window and it can close it. */
void (*deprecated_readline_begin_hook) (char *, ...);
-char *(*deprecated_readline_hook) (char *);
+char *(*deprecated_readline_hook) (const char *);
void (*deprecated_readline_end_hook) (void);
/* Called as appropriate to notify the interface that we have attached
@@ -620,7 +620,7 @@ prevent_dont_repeat (void)
A NULL return means end of file. */
char *
-gdb_readline (char *prompt_arg)
+gdb_readline (const char *prompt_arg)
{
int c;
char *result;
@@ -812,7 +812,7 @@ gdb_readline_wrapper_cleanup (void *arg)
}
char *
-gdb_readline_wrapper (char *prompt)
+gdb_readline_wrapper (const char *prompt)
{
struct cleanup *back_to;
struct gdb_readline_wrapper_cleanup *cleanup;
@@ -912,14 +912,14 @@ gdb_rl_operate_and_get_next (int count, int key)
simple input as the user has requested. */
char *
-command_line_input (char *prompt_arg, int repeat, char *annotation_suffix)
+command_line_input (const char *prompt_arg, int repeat, char *annotation_suffix)
{
static char *linebuffer = 0;
static unsigned linelength = 0;
char *p;
char *p1;
char *rl;
- char *local_prompt = prompt_arg;
+ char *local_prompt;
char *nline;
char got_eof = 0;
@@ -938,6 +938,8 @@ command_line_input (char *prompt_arg, int repeat, char *annotation_suffix)
strcat (local_prompt, "\n\032\032");
strcat (local_prompt, annotation_suffix);
strcat (local_prompt, "\n");
+
+ prompt_arg = local_prompt;
}
if (linebuffer == 0)
@@ -979,15 +981,15 @@ command_line_input (char *prompt_arg, int repeat, char *annotation_suffix)
/* Don't use fancy stuff if not talking to stdin. */
if (deprecated_readline_hook && input_from_terminal_p ())
{
- rl = (*deprecated_readline_hook) (local_prompt);
+ rl = (*deprecated_readline_hook) (prompt_arg);
}
else if (command_editing_p && input_from_terminal_p ())
{
- rl = gdb_readline_wrapper (local_prompt);
+ rl = gdb_readline_wrapper (prompt_arg);
}
else
{
- rl = gdb_readline (local_prompt);
+ rl = gdb_readline (prompt_arg);
}
if (annotation_level > 1 && instream == stdin)
@@ -1021,7 +1023,7 @@ command_line_input (char *prompt_arg, int repeat, char *annotation_suffix)
break;
p--; /* Put on top of '\'. */
- local_prompt = (char *) 0;
+ prompt_arg = NULL;
}
#ifdef STOP_SIGNAL