This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Fix leak in most gdb.mi tests.
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