From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com [209.85.128.43]) by sourceware.org (Postfix) with ESMTPS id EB96D3877204 for ; Fri, 16 Dec 2022 14:11:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EB96D3877204 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wm1-f43.google.com with SMTP id v124-20020a1cac82000000b003cf7a4ea2caso4181089wme.5 for ; Fri, 16 Dec 2022 06:11:50 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=4FmIJ1z2EF6HKz6UVrcfk4l1cMiv6PPmrNwFfoFcyjc=; b=lz6fDkclpH97kEJl8wYAlkedOjmSOfWBRhXVdPn3PSKusq9sSChQTw+py1NRE19DAR jrbOUC7eqVBTD76CWSWiKXYwo6KeyguIyczsZzCv4J9ElQ1qadA4v4vFe4FdSVSInj/J FNVyoW6lWRpAnCtO7HxGjv7HHyLPF97jQV/koDCFk75V2pn83lsb6b5PxvlUsrqlFDhp KaWH6jpAJUcJZEP2HsOgoU1fxy7yczh+tfzqUncuQHOfZH8t0vaCBaABiFQMJbu7jKd/ NtWJ5y4OuXEUC5u5qoXz2WawUbv4ZBKtVkJS97IlA0/+2DQZf6IXUt6djI1QO0N6/Jhv tnCA== X-Gm-Message-State: ANoB5pl20oIDR0OrKXg1M8DfUzssmVXNeoAGGqXqlrpb0LrtNVDVMvMh xbmo6uSNpFCCsewOUjQniwc= X-Google-Smtp-Source: AA0mqf4N4QyUypisI3FojZ7WxlosMjjRSIGMgDKQdAIQF51NgxDBt23nlkehlhLH0jOubdUNt8eRbA== X-Received: by 2002:a05:600c:500a:b0:3d2:3ca2:2d4f with SMTP id n10-20020a05600c500a00b003d23ca22d4fmr8076696wmr.36.1671199909729; Fri, 16 Dec 2022 06:11:49 -0800 (PST) Received: from ?IPv6:2001:8a0:f912:6700:afd9:8b6d:223f:6170? ([2001:8a0:f912:6700:afd9:8b6d:223f:6170]) by smtp.gmail.com with ESMTPSA id p12-20020a05600c468c00b003cf483ee8e0sm11831859wmo.24.2022.12.16.06.11.48 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 16 Dec 2022 06:11:49 -0800 (PST) Subject: Re: [PATCH RESEND] gdb: remove static buffer in command_line_input To: Simon Marchi , gdb-patches@sourceware.org Cc: Jan Vrany References: <20221215190625.758546-1-simon.marchi@polymtl.ca> <1e61d9cc-00e0-56d6-c232-96d465a78782@palves.net> <8a091e68-2609-b28d-3945-9b763ed9b6d0@polymtl.ca> From: Pedro Alves Message-ID: Date: Fri, 16 Dec 2022 14:11:48 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <8a091e68-2609-b28d-3945-9b763ed9b6d0@polymtl.ca> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.7 required=5.0 tests=BAYES_00,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 2022-12-16 1:38 p.m., Simon Marchi wrote: > > > On 12/16/22 08:21, Pedro Alves wrote: >> On 2022-12-15 7:06 p.m., Simon Marchi via Gdb-patches wrote: >>> else >>> { >>> /* Copy whole line including terminating null, and we're >>> done. */ >>> - buffer_grow (cmd_line_buffer, rl, len + 1); >>> - cmd = cmd_line_buffer->buffer; >>> + cmd_line_buffer.append (rl, len + 1); >>> + return true; >> >> std::string is guaranteed to be null terminated, but, it can also hold >> \0 characters in the middle of the string. Isn't that >> >> cmd_line_buffer.append (rl, len + 1); >> >> ending up with an embedded \0 at the end of the string, with cmd_line_buffer.size() accounting for it? > > Argh, yes! I rewrote this patch from scratch multiple times until I got > something satisfactory, and I know I changed that in a previous > iteration, because I remember having that thought. > > Here's a patch to fix it: > > > From 8757d2ec9edf2ae0b77039ca0ecc9ca32fd87256 Mon Sep 17 00:00:00 2001 > From: Simon Marchi > Date: Fri, 16 Dec 2022 08:34:56 -0500 > Subject: [PATCH] gdb: don't copy final null character in > command_line_append_input_line > > The modified std::string::append call currently copies the null > character from the source string, leaving the std::string with a null > character in its content, part of its size, in addition to the null > character managed by the std::string itself We don't want to copy that Missing period after "itself". > null character, so remove that "+ 1". > > Change-Id: I412fe9bd6b08e7ce7604ef5d8038b62f1a784c9b > --- > gdb/event-top.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gdb/event-top.c b/gdb/event-top.c > index fa86a89e4a19..fa9e1f3f6a73 100644 > --- a/gdb/event-top.c > +++ b/gdb/event-top.c > @@ -640,7 +640,7 @@ command_line_append_input_line (std::string &cmd_line_buffer, const char *rl) > { > /* Copy whole line including terminating null, and we're > done. */ > - cmd_line_buffer.append (rl, len + 1); > + cmd_line_buffer.append (rl, len); I'd update the comment -- the "including terminating null" part seems misleading now, just drop that part? Otherwise LTGM. > return true; > } > } > > base-commit: e60a615dde5d6674a6488b74afe807a775551407 >