From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 59893 invoked by alias); 3 Jan 2018 19:13:34 -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 59303 invoked by uid 89); 3 Jan 2018 19:13:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=enforced X-Spam-User: qpsmtpd, 2 recipients 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, 03 Jan 2018 19:13:32 +0000 Received: by simark.ca (Postfix, from userid 112) id B8EA51E5A6; Wed, 3 Jan 2018 14:13:30 -0500 (EST) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id DC6BA1E519; Wed, 3 Jan 2018 14:13:28 -0500 (EST) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Wed, 03 Jan 2018 19:13:00 -0000 From: Simon Marchi To: John Baldwin Cc: gdb-patches@sourceware.org, binutils@sourceware.org Subject: Re: [PATCH 3/4] Support 'info proc' for native FreeBSD processes. In-Reply-To: <137803623.10H6MxTIBr@ralph.baldwin.cx> References: <20171222220513.54983-1-jhb@FreeBSD.org> <20171222220513.54983-4-jhb@FreeBSD.org> <137803623.10H6MxTIBr@ralph.baldwin.cx> Message-ID: X-Sender: simark@simark.ca User-Agent: Roundcube Webmail/1.3.2 X-SW-Source: 2018-01/txt/msg00037.txt.bz2 Hi John, >> > +#ifdef HAVE_KINFO_GETVMMAP >> > + if (do_mappings) >> > + { >> > + int nvment; >> > + std::unique_ptr> >> >> Is there a reason to have and use free_deleter rather than >> gdb::unique_xmalloc_ptr? >> >> > + vmentl (kinfo_getvmmap (pid, &nvment)); > > This function (kinfo_getvmmap) which is defined in the libutil library > included in > FreeBSD's base system calls malloc() internally, so the memory returned > must be > freed with free() rather than xfree(). This deleter is already used > earlier in > fbsd_find_memory_regions() for another call to kinfo_getvmmap() for > the same reason. But isn't xfree just a wrapper around free? >> > + >> > + if (vmentl) >> >> vmentl != NULL >> >> There are a few other instances of if (ptr) that should be changed to >> if (ptr != NULL). > > Ok. The style in GDB doesn't seem consistent in this regard (I > largely based this > code on the 'info proc' implementation in linux-tdep.c which doesn't > explicitly > compare against NULL/nullptr (though I prefer the explicit comparison > myself)). > Also, I feel like we should use nullptr rather than NULL when working > with "smart" > pointer types like unique_ptr<> at least? You are right, there is code that comes from an era where the coding style wasn't as strictly enforced, it seems. I don't think we should go fix coding style issues just for fun, but when we modify or copy existing code, we should make it respect the style. Thanks, Simon