[RFA] Fix leak in most gdb.mi tests.

  >>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> Maybe input_handler could accept a unique_xmalloc_ptr<char>&& instead.
Tom> That would at least clarify the ownership semantics.

What do you think of this?

Tom

commit fe70f92b8e3c0af79d041b9004b9abbeed27374d
Author: Tom Tromey <tom@tromey.com>
Date:   Sat Dec 29 12:42:18 2018 -0700

    Change input_handler to take a unique_xmalloc_ptr
    
    This changes ui::input_handler to take a unique_xmalloc_ptr.  This
    clarifies the ownership transfer of input_handler's argument.
    
    gdb/ChangeLog
    2018-12-29  Tom Tromey  <tom@tromey.com>
    
            * event-top.h (command_line_handler): Update.
            * top.c (class gdb_readline_wrapper_cleanup) <m_handler_orig>:
            Update.
            (gdb_readline_wrapper_line): Update.
            * top.h (struct ui) <input_handler>: Take a unique_xmalloc_ptr.
            (handle_line_of_input): Update.
            * event-top.c: Update.
            (gdb_readline_no_editing_callback): Update.
            (command_line_handler): Take a unique_xmalloc_ptr.
            (handle_line_of_input): Take a const char *.
            (command_line_append_input_line): Take a const char *.
  

On Sat, 2018-12-29 at 12:44 -0700, Tom Tromey wrote:
> > > > > > "Tom" == Tom Tromey <tom@tromey.com> writes:
> 
> Tom> Maybe input_handler could accept a unique_xmalloc_ptr<char>&& instead.
> Tom> That would at least clarify the ownership semantics.
> 
> What do you think of this?

The patch looks nice to me.  I re-run all gdb.mi tests under valgrind,
and no errors/no leak of the line_buffer.
A minor comment below.

Thanks

Philippe
> 
> Tom
> 
> commit fe70f92b8e3c0af79d041b9004b9abbeed27374d
> Author: Tom Tromey <tom@tromey.com>
> Date:   Sat Dec 29 12:42:18 2018 -0700
> 
>     Change input_handler to take a unique_xmalloc_ptr
>     
>     This changes ui::input_handler to take a unique_xmalloc_ptr.  This
>     clarifies the ownership transfer of input_handler's argument.
>     
>     gdb/ChangeLog
>     2018-12-29  Tom Tromey  <tom@tromey.com>
>     
>             * event-top.h (command_line_handler): Update.
>             * top.c (class gdb_readline_wrapper_cleanup) <m_handler_orig>:
>             Update.
>             (gdb_readline_wrapper_line): Update.
>             * top.h (struct ui) <input_handler>: Take a unique_xmalloc_ptr.
>             (handle_line_of_input): Update.
>             * event-top.c: Update.
>             (gdb_readline_no_editing_callback): Update.
>             (command_line_handler): Take a unique_xmalloc_ptr.
>             (handle_line_of_input): Take a const char *.
>             (command_line_append_input_line): Take a const char *.
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 5ed91a84fc..56d86acee3 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,17 @@
> +2018-12-29  Tom Tromey  <tom@tromey.com>
> +
> +	* event-top.h (command_line_handler): Update.
> +	* top.c (class gdb_readline_wrapper_cleanup) <m_handler_orig>:
> +	Update.
> +	(gdb_readline_wrapper_line): Update.
> +	* top.h (struct ui) <input_handler>: Take a unique_xmalloc_ptr.
> +	(handle_line_of_input): Update.
> +	* event-top.c: Update.
> +	(gdb_readline_no_editing_callback): Update.
> +	(command_line_handler): Take a unique_xmalloc_ptr.
> +	(handle_line_of_input): Take a const char *.
> +	(command_line_append_input_line): Take a const char *.
> +
>  2018-12-28  Tom Tromey  <tom@tromey.com>
>  
>  	* source-cache.c (get_language_name): Conditionally compile.
> diff --git a/gdb/event-top.c b/gdb/event-top.c
> index 5852089f09..fd7c869bc0 100644
> --- a/gdb/event-top.c
> +++ b/gdb/event-top.c
> @@ -210,7 +210,7 @@ gdb_rl_callback_handler (char *rl) noexcept
>  
>    TRY
>      {
> -      ui->input_handler (rl);
> +      ui->input_handler (gdb::unique_xmalloc_ptr<char> (rl));
>      }
>    CATCH (ex, RETURN_MASK_ALL)
>      {
> @@ -594,7 +594,7 @@ command_handler (const char *command)
>     line ends in a backslash).  Takes ownership of RL.  */
Remove the part of the comment about ownership ?

>  
>  static char *
> -command_line_append_input_line (struct buffer *cmd_line_buffer, char *rl)
> +command_line_append_input_line (struct buffer *cmd_line_buffer, const char *rl)
>  {
>    char *cmd;
>    size_t len;
> @@ -615,9 +615,6 @@ command_line_append_input_line (struct buffer *cmd_line_buffer, char *rl)
>        cmd = cmd_line_buffer->buffer;
>      }
>  
> -  /* Allocated in readline.  */
> -  xfree (rl);
> -
>    return cmd;
>  }
>  
> @@ -643,7 +640,8 @@ command_line_append_input_line (struct buffer *cmd_line_buffer, char *rl)
>  
>  char *
>  handle_line_of_input (struct buffer *cmd_line_buffer,
> -		      char *rl, int repeat, const char *annotation_suffix)
> +		      const char *rl, int repeat,
> +		      const char *annotation_suffix)
>  {
>    struct ui *ui = current_ui;
>    int from_tty = ui->instream == ui->stdin_stream;
> @@ -746,13 +744,13 @@ handle_line_of_input (struct buffer *cmd_line_buffer,
>     function.  */
>  
>  void
> -command_line_handler (char *rl)
> +command_line_handler (gdb::unique_xmalloc_ptr<char> &&rl)
>  {
>    struct buffer *line_buffer = get_command_line_buffer ();
>    struct ui *ui = current_ui;
>    char *cmd;
>  
> -  cmd = handle_line_of_input (line_buffer, rl, 1, "prompt");
> +  cmd = handle_line_of_input (line_buffer, rl.get (), 1, "prompt");
>    if (cmd == (char *) EOF)
>      {
>        /* stdin closed.  The connection with the terminal is gone.
> @@ -846,7 +844,7 @@ gdb_readline_no_editing_callback (gdb_client_data client_data)
>  
>    buffer_grow_char (&line_buffer, '\0');
>    result = buffer_finish (&line_buffer);
> -  ui->input_handler (result);
> +  ui->input_handler (gdb::unique_xmalloc_ptr<char> (result));
>  }
>  
>  
> diff --git a/gdb/event-top.h b/gdb/event-top.h
> index a77303e5c3..1164f69071 100644
> --- a/gdb/event-top.h
> +++ b/gdb/event-top.h
> @@ -35,7 +35,7 @@ extern void gdb_disable_readline (void);
>  extern void async_init_signals (void);
>  extern void change_line_handler (int);
>  
> -extern void command_line_handler (char *rl);
> +extern void command_line_handler (gdb::unique_xmalloc_ptr<char> &&rl);
>  extern void command_handler (const char *command);
>  
>  #ifdef SIGTSTP
> diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
> index 9a317bc0ec..fd446c20e6 100644
> --- a/gdb/mi/mi-interp.c
> +++ b/gdb/mi/mi-interp.c
> @@ -43,7 +43,8 @@
>     interpreter.  */
>  
>  static void mi_execute_command_wrapper (const char *cmd);
> -static void mi_execute_command_input_handler (char *cmd);
> +static void mi_execute_command_input_handler
> +  (gdb::unique_xmalloc_ptr<char> &&cmd);
>  
>  /* These are hooks that we put in place while doing interpreter_exec
>     so we can report interesting things that happened "behind the MI's
> @@ -294,14 +295,14 @@ mi_on_sync_execution_done (void)
>  /* mi_execute_command_wrapper wrapper suitable for INPUT_HANDLER.  */
>  
>  static void
> -mi_execute_command_input_handler (char *cmd)
> +mi_execute_command_input_handler (gdb::unique_xmalloc_ptr<char> &&cmd)
>  {
>    struct mi_interp *mi = as_mi_interp (top_level_interpreter ());
>    struct ui *ui = current_ui;
>  
>    ui->prompt_state = PROMPT_NEEDED;
>  
> -  mi_execute_command_wrapper (cmd);
> +  mi_execute_command_wrapper (cmd.get ());
>  
>    /* Print a prompt, indicating we're ready for further input, unless
>       we just started a synchronous command.  In that case, we're about
> diff --git a/gdb/top.c b/gdb/top.c
> index 4bcb4e28fb..8b3fc5ee9a 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -900,10 +900,10 @@ gdb_in_secondary_prompt_p (struct ui *ui)
>     text.  */
>  
>  static void
> -gdb_readline_wrapper_line (char *line)
> +gdb_readline_wrapper_line (gdb::unique_xmalloc_ptr<char> &&line)
>  {
>    gdb_assert (!gdb_readline_wrapper_done);
> -  gdb_readline_wrapper_result = line;
> +  gdb_readline_wrapper_result = line.release ();
>    gdb_readline_wrapper_done = 1;
>  
>    /* Prevent operate-and-get-next from acting too early.  */
> @@ -972,7 +972,7 @@ public:
>  
>  private:
>  
> -  void (*m_handler_orig) (char *);
> +  void (*m_handler_orig) (gdb::unique_xmalloc_ptr<char> &&);
>    int m_already_prompted_orig;
>  
>    /* Whether the target was async.  */
> diff --git a/gdb/top.h b/gdb/top.h
> index b34defa1f2..4d11f3443b 100644
> --- a/gdb/top.h
> +++ b/gdb/top.h
> @@ -81,7 +81,7 @@ struct ui
>  
>    /* The function to invoke when a complete line of input is ready for
>       processing.  */
> -  void (*input_handler) (char *);
> +  void (*input_handler) (gdb::unique_xmalloc_ptr<char> &&);
>  
>    /* True if this UI is using the readline library for command
>       editing; false if using GDB's own simple readline emulation, with
> @@ -297,7 +297,7 @@ extern void show_history (const char *, int);
>  extern void set_verbose (const char *, int, struct cmd_list_element *);
>  
>  extern char *handle_line_of_input (struct buffer *cmd_line_buffer,
> -				   char *rl, int repeat,
> +				   const char *rl, int repeat,
>  				   const char *annotation_suffix);
>  
>  #endif
  

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> The patch looks nice to me.  I re-run all gdb.mi tests under valgrind,
Philippe> and no errors/no leak of the line_buffer.
Philippe> A minor comment below.

Thanks for taking a look.

>> @@ -594,7 +594,7 @@ command_handler (const char *command)
>> line ends in a backslash).  Takes ownership of RL.  */

Philippe> Remove the part of the comment about ownership ?

I'm checking it in with this change.

Tom