From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f45.google.com (mail-wm1-f45.google.com [209.85.128.45]) by sourceware.org (Postfix) with ESMTPS id DDF8E3858D20 for ; Tue, 11 Apr 2023 12:58:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DDF8E3858D20 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-f45.google.com with SMTP id n19-20020a05600c501300b003f064936c3eso9296712wmr.0 for ; Tue, 11 Apr 2023 05:58:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1681217885; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:to:subject :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=bIdDE2S7GIAJ6c6PFfh+KmL+FNcU8PNLahcYB53bv0M=; b=MXi3sl6+6v0qJ3enQOzGJpE50eTNOz8mP/s9Shm0cqAdtVYxesjvyPmwH2Py64UN/K cVsViumB4gHXUZZ9D0bQMF84LbFYV2nk3Jk2KTxFXU3+zuhlAijqqUFdHC9//M7CcJnd lKO+5biPNamrrSvkyRbgjZ14y7vDtsi7DdtNTBbT26C47cHHmFMLBPcuFOwbY37ez1X3 rk8TOboedpaCHmSdwCri+QiAIuxtOmDpp9ZCyozdAJVHkSXuHk2gpI3MgOvxQmoPamsL ghjeFSB6t3Ql6MhPmDHAfraOMEB8ubMjuuNvNQcmCrOsJQqlldBnx3ytanuNp1atDZ5L T0dg== X-Gm-Message-State: AAQBX9ecF5Hqbpli6U1QItlqRAGTwPOZJQV8I7aF40jpryIMn4tZ+R8/ vYw2ogYf0ObD05MnLaRdgXU3qmN3vGCvHw== X-Google-Smtp-Source: AKy350YAZPxI33x6k9yLEk1D5VekEHiAIAINIyFGhgJD2l8orMkLuOvcK4ggyNCtpfqSbLoTjxMWrw== X-Received: by 2002:a7b:c006:0:b0:3f0:7db5:6078 with SMTP id c6-20020a7bc006000000b003f07db56078mr9328809wmb.27.1681217885255; Tue, 11 Apr 2023 05:58:05 -0700 (PDT) Received: from ?IPv6:2001:8a0:f90a:9800::1fe? ([2001:8a0:f90a:9800::1fe]) by smtp.gmail.com with ESMTPSA id h7-20020a05600c350700b003ee9f396dcesm21116758wmq.30.2023.04.11.05.58.04 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 11 Apr 2023 05:58:04 -0700 (PDT) Subject: Re: [PATCH] gdb: fix missing null-character when using value_cstring To: Andrew Burgess , Simon Marchi , gdb-patches@sourceware.org References: <3d7197c2-424c-1458-93aa-d23fedac3d70@simark.ca> <87r0sxb0zy.fsf@redhat.com> From: Pedro Alves Message-ID: <56b5d8f3-463e-7a73-3cf6-4da661172d4f@palves.net> Date: Tue, 11 Apr 2023 13:58:06 +0100 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: <87r0sxb0zy.fsf@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.8 required=5.0 tests=BAYES_00,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,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 2023-04-06 2:20 p.m., Andrew Burgess via Gdb-patches wrote: > Simon Marchi writes: > >> On 4/3/23 17:49, Andrew Burgess via Gdb-patches wrote: >>> Alternatively, if we change the requirements of value_cstring so that >>> we require the passed length includes the trailing null, then we can >>> assert that this is indeed the case within value_cstring. Any >>> incorrect uses in the future will be quickly spotted. >>> >>> So that's what I did, c_string_operation::evaluate is left unchanged, >>> but every other use of value_cstring is adjusted with a '+ 1' so that >>> we include the null within the length being passed. >> >> That sounds counterintuitive to me. With an API of style pointer + >> length, I don't expect the length to include the null terminator. It >> also unnecessarily forces the caller to have a null-terminated version >> of the string, which may not always be the case (you might want to call >> value_cstring on a subset of an existing string). >> >> I think that: >> >> struct value * >> value_cstring (const char *ptr, ssize_t len, struct type *char_type) >> >> should take a length excluding the null terminator, but a null >> terminator in the result (its job is to build a C string, and a C string >> requires a null terminator at the end). > > This is why writing comments is so important :) > > I read it as "build a value* from this C-string", which is why I figured > we can assume there will be a '\0' at the end. > > Anyway, I don't really mind either way, just so long as we can get > something that works! I'll flip this around to the way you suggest and > repost. > Hmm, this rang a bell. See this patch and following discussion: https://inbox.sourceware.org/gdb-patches/20210713153142.3957666-1-simon.marchi@efficios.com/ Pedro Alves