Re: [PATCH] Fix build with Python 3.4 (PR python/16784)

[Simon Marchi <simon dot marchi at ericsson dot com>]

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"
> 
>> 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

Hi Pedro,

Here is the v2, with changes according to your comments.

>From 8ccae8f39992e7515711d6fc193f1d68cb47eaf8 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Tue, 25 Nov 2014 21:02:15 -0500
Subject: [PATCH v2] Fix build with Python 3.4 (PR python/16784)

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 went 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 adjust gdb accordingly by making the prompt_arg parameter
const, as well as the fallouts of that. I needed to rework how
annotations are added to the prompt, since the it is now const. If
annotations are enabled, it will make a copy of the prompt overwrite the
prompt variable that is used throughout the function. Otherwise, no copy
is done and the original prompt_arg value is passed.

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.x, 3.3.y and 3.4.z.

New in v2:
* Formatting.
* Don't change the prompt_arg argument, use a local variable instead.
* Move local_prompt in a narrower scope.

Signed-off-by: Simon Marchi <simon.marchi@ericsson.com>
---
 gdb/defs.h                   |  8 ++++----
 gdb/event-top.c              |  2 +-
 gdb/event-top.h              |  2 +-
 gdb/python/py-gdb-readline.c |  4 ++++
 gdb/top.c                    | 30 +++++++++++++++++-------------
 5 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/gdb/defs.h b/gdb/defs.h
index 6b63b6a..24e1e3b 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -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);
diff --git a/gdb/event-top.c b/gdb/event-top.c
index cb438ac..55caf72 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -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;
diff --git a/gdb/event-top.h b/gdb/event-top.h
index 919287e..0c8baae 100644
--- a/gdb/event-top.h
+++ b/gdb/event-top.h
@@ -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);
diff --git a/gdb/python/py-gdb-readline.c b/gdb/python/py-gdb-readline.c
index d98a196..ab419bc 100644
--- a/gdb/python/py-gdb-readline.c
+++ b/gdb/python/py-gdb-readline.c
@@ -29,7 +29,11 @@

 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;
diff --git a/gdb/top.c b/gdb/top.c
index 83d858a..7f38562 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -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;
+  const char *prompt = prompt_arg;
   char *p;
   char *p1;
   char *rl;
-  char *local_prompt = prompt_arg;
   char *nline;
   char got_eof = 0;

@@ -929,15 +929,19 @@ command_line_input (char *prompt_arg, int repeat, char *annotation_suffix)

   if (annotation_level > 1 && instream == stdin)
     {
-      local_prompt = alloca ((prompt_arg == NULL ? 0 : strlen (prompt_arg))
+      char *local_prompt;
+
+      local_prompt = alloca ((prompt == NULL ? 0 : strlen (prompt))
 			     + strlen (annotation_suffix) + 40);
-      if (prompt_arg == NULL)
+      if (prompt == NULL)
 	local_prompt[0] = '\0';
       else
-	strcpy (local_prompt, prompt_arg);
+	strcpy (local_prompt, prompt);
       strcat (local_prompt, "\n\032\032");
       strcat (local_prompt, annotation_suffix);
       strcat (local_prompt, "\n");
+
+      prompt = local_prompt;
     }

   if (linebuffer == 0)
@@ -979,15 +983,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);
 	}
       else if (command_editing_p && input_from_terminal_p ())
 	{
-	  rl = gdb_readline_wrapper (local_prompt);
+	  rl = gdb_readline_wrapper (prompt);
 	}
       else
 	{
-	  rl = gdb_readline (local_prompt);
+	  rl = gdb_readline (prompt);
 	}

       if (annotation_level > 1 && instream == stdin)
@@ -1021,7 +1025,7 @@ command_line_input (char *prompt_arg, int repeat, char *annotation_suffix)
 	break;

       p--;			/* Put on top of '\'.  */
-      local_prompt = (char *) 0;
+      prompt = NULL;
     }

 #ifdef STOP_SIGNAL
@@ -1064,7 +1068,7 @@ command_line_input (char *prompt_arg, int repeat, char *annotation_suffix)
 	  if (expanded < 0)
 	    {
 	      xfree (history_value);
-	      return command_line_input (prompt_arg, repeat,
+	      return command_line_input (prompt, repeat,
 					 annotation_suffix);
 	    }
 	  if (strlen (history_value) > linelength)
-- 
2.1.3