From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 60445 invoked by alias); 21 Feb 2018 00:07:31 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 60417 invoked by uid 89); 21 Feb 2018 00:07:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=calss, H*r:sk:22.2018 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-oi0-f44.google.com Received: from mail-oi0-f44.google.com (HELO mail-oi0-f44.google.com) (209.85.218.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 21 Feb 2018 00:07:29 +0000 Received: by mail-oi0-f44.google.com with SMTP id t185so5742858oif.6; Tue, 20 Feb 2018 16:07:29 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=rySWiQHqM1jBuyrNsguGzr2BG4hKMAqkxByklx3iM/w=; b=M13xjI8XmyWOWUG3xxhUVBKZTIhtLP3ATkK3jNmi1xlNTcWghLsfj5mRygdwze01NW BhNIlqh2xs0LUNez5sPOTzQNpOXH3armslKMgSZNHRf9eSVdHjRKeYsNuz8BCp0lO9V7 MFqOWCz3l3fhLLULRcOm6a8OZcj3tdZsPSrYoCHIgsAFYa8z/tMUmjlSGCcr08/0VgdJ IRdp7rlEibWCMqTvYyU2HvVhCxTBps3ffCn633YwX76E56S4Bm6x3ghFmzVwU5bP0/wN E6szWDG9AJZdXAMDF9GlywHhEEdIcSpXu1nBxojuk+oP4S8xJdQ/OICrTXtBQoyPTZdY NHQw== X-Gm-Message-State: APf1xPDk54rmt38SSrAEdI2DOS+0gk3Eqve8JWG9GoMY760mQQMEYCKW ShtUGUZC9vgt3HXQCI78KmWkmQ== X-Google-Smtp-Source: AH8x224H8evQXTxby2g6YT82N7NStSTKnFMeMXqxW4aHTQ1QIDLetAomPPIB3fl6572btDo02gxayg== X-Received: by 10.202.60.69 with SMTP id j66mr912655oia.222.1519171647340; Tue, 20 Feb 2018 16:07:27 -0800 (PST) Received: from localhost.localdomain (75-171-228-29.hlrn.qwest.net. [75.171.228.29]) by smtp.gmail.com with ESMTPSA id y56sm14761250oty.22.2018.02.20.16.07.25 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 20 Feb 2018 16:07:26 -0800 (PST) Subject: Re: RFA: Sanitize deprecation messages (PR 84195) To: Nick Clifton , David Malcolm , msebor@gcc.gnu.org References: <87h8qv9xhb.fsf@redhat.com> <79784243-bf45-167f-a546-55c892ea02e9@gmail.com> <93d668b3-30d1-e898-544f-19a153325e03@redhat.com> <1518031052.26503.90.camel@redhat.com> <0d7e6077-0d67-0c7b-5058-fc66e7c18d87@gmail.com> <1518728685.2913.14.camel@redhat.com> <490266ba-d4c5-2016-6a0f-bc0178f834a0@gmail.com> <8ed3cc4d-82cc-4c99-8dbe-e18c949d5c6f@redhat.com> Cc: dodji@redhat.com, gcc-patches@gcc.gnu.org From: Martin Sebor Message-ID: Date: Wed, 21 Feb 2018 00:07:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <8ed3cc4d-82cc-4c99-8dbe-e18c949d5c6f@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2018-02/txt/msg01205.txt.bz2 On 02/20/2018 05:48 AM, Nick Clifton wrote: > Hi Martin, > > >> Since the class manages a resource it should ideally make sure it >> doesn't try to release the same resource multiple times. I.e., its >> copy constructor and assignment operators should either "do the right >> thing" (whatever you think that is) or be made inaccessible (or declared deleted in C++ 11). >> >> For example: >> >> { >> escaped_string a; >> a.escape ("foo\nbar"); >> >> escaped_string b (a); >> // b destroys its m_str >> // double free in a's destructor here >> } > > I am not sure that this can happen. First of the escaped_string > class does not have constructor that accepts a char* argument. > (Maybe in C++ this is done automatically ? My C++-fu is very weak). It doesn't have a converting ctor and the above example doesn't use one. It uses the implicitly defined copy constructor to create a copy of A in B. > > Secondly the m_owned private field is only set to true when > the m_str field is set to a string allocated by the particular > instance of the class, and memory is only freed by the destructor > if m_owned is true. Right, and implicitly-defined copy constructor copies all the fields so both the original and the copy wind up pointing to the same allocated chunk and both with m_owned set to true. The same thing happens when an object of the calss is assigned to another, thanks to the implicitly-defined copy assignment operator. > So even this should work: > > { > escaped_string a,b; > > a.escape ("foo\nbar"); > b.escape ((const char *) a); > } Yes, this works but this wouldn't: { escaped_string a, b; a.escape ("foo\nbar"); b = a; } // double free in a's dtor here What also wouldn't work right is the converse: { escaped_string a, b; a.escape ("foo\nbar"); a = b; // a.m_str leaks here (it's reset by the assignment) } Your patch works correctly because it doesn't use the copy constructor or the copy assignment operator. So this is only a hypothetical concern, if a change in the code led to one of the copy functions being called. Hope this makes sense. Martin > > The destructor for B will not free any memory, even though its > m_str field has been set to the same string as A, because its > m_owned field will be set to FALSE. > > Cheers > Nick >