From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 38065 invoked by alias); 19 Oct 2016 18:25:47 -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 38052 invoked by uid 89); 19 Oct 2016 18:25:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.2 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=no version=3.3.2 spammy=H*r:112, safer, H*u:1.2.0, H*UA:1.2.0 X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 19 Oct 2016 18:25:44 +0000 Received: by simark.ca (Postfix, from userid 112) id 033DB1E80F; Wed, 19 Oct 2016 14:25:43 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id E65FB1E0F5; Wed, 19 Oct 2016 14:25:41 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Wed, 19 Oct 2016 18:25:00 -0000 From: Simon Marchi To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v2 04/31] cli-script.c: Simplify using std::string, eliminate cleanups In-Reply-To: <1476839539-8374-5-git-send-email-palves@redhat.com> References: <1476839539-8374-1-git-send-email-palves@redhat.com> <1476839539-8374-5-git-send-email-palves@redhat.com> Message-ID: <72d887888ab5002b79165c861734d6d2@simark.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.2.0 X-IsSubscribed: yes X-SW-Source: 2016-10/txt/msg00579.txt.bz2 On 2016-10-18 21:11, Pedro Alves wrote: > @@ -457,12 +456,13 @@ execute_control_command (struct command_line > *cmd) > switch (cmd->control_type) > { > case simple_control: > - /* A simple command, execute it and return. */ > - new_line = insert_args (cmd->line); > - make_cleanup (free_current_contents, &new_line); > - execute_command (new_line, 0); > - ret = cmd->control_type; > - break; > + { > + /* A simple command, execute it and return. */ > + std::string new_line = insert_args (cmd->line); > + execute_command (&new_line[0], 0); The need to use &new_line[0] instead of .c_str() here looks (or smells) like a code smell to me. If execute_command needs to modify the string, it should either make its own copy, or at least document in what ways it can be modified. And in general, modifying an std::string's underlying array is probably not good (if I understand the standard correctly, it's undefined behaviour, though in reality it probably always works). Do you know of any caller of execute_command that relies on the modification that execute_command does to the passed string? If not, I think it would be safer to change the arg to a const char * and duplicate the string at function entry.