From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 103480 invoked by alias); 30 Dec 2018 09:28:33 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 103471 invoked by uid 89); 30 Dec 2018 09:28:33 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-27.6 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mailsec102.isp.belgacom.be Received: from mailsec102.isp.belgacom.be (HELO mailsec102.isp.belgacom.be) (195.238.20.98) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 30 Dec 2018 09:28:30 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=skynet.be; i=@skynet.be; q=dns/txt; s=securemail; t=1546162110; x=1577698110; h=message-id:subject:from:to:cc:date:in-reply-to: references:mime-version:content-transfer-encoding; bh=5Ev0Ic+f1k+KccgNgzW2cVaK1nJPitUXh92FkT/y/7I=; b=TlzZcSBdaQTs03rYymicH9gbtcpL6y8cHejZ/gtCZSvczcVVU7ERv+2Q RdAFaBqOtWmaXlNSwyTealYcSttW9Q==; Received: from 184.205-67-87.adsl-dyn.isp.belgacom.be (HELO md) ([87.67.205.184]) by relay.skynet.be with ESMTP/TLS/AES256-GCM-SHA384; 30 Dec 2018 10:28:28 +0100 Message-ID: <1546162107.12900.3.camel@skynet.be> Subject: Re: [RFA] Fix leak in most gdb.mi tests. From: Philippe Waroquiers To: Tom Tromey Cc: gdb-patches@sourceware.org Date: Sun, 30 Dec 2018 09:28:00 -0000 In-Reply-To: <87wonsktj2.fsf@tromey.com> References: <20181229170116.8588-1-philippe.waroquiers@skynet.be> <87a7kom9ez.fsf@tromey.com> <871s60m8lg.fsf@tromey.com> <87wonsktj2.fsf@tromey.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2018-12/txt/msg00430.txt.bz2 On Sat, 2018-12-29 at 12:44 -0700, Tom Tromey wrote: > > > > > > "Tom" == Tom Tromey writes: > > Tom> Maybe input_handler could accept a unique_xmalloc_ptr&& 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 > 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 > > * event-top.h (command_line_handler): Update. > * top.c (class gdb_readline_wrapper_cleanup) : > Update. > (gdb_readline_wrapper_line): Update. > * top.h (struct ui) : 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 > + > + * event-top.h (command_line_handler): Update. > + * top.c (class gdb_readline_wrapper_cleanup) : > + Update. > + (gdb_readline_wrapper_line): Update. > + * top.h (struct ui) : 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 > > * 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 (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 &&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 (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 &&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 &&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 &&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 &&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 &&); > 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 &&); > > /* 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