From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11891 invoked by alias); 3 Jan 2018 19:05:54 -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 11817 invoked by uid 89); 3 Jan 2018 19:05:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.7 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=no version=3.3.2 spammy=46123, H*F:D*freebsd.org, Hx-languages-length:2195 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail.baldwin.cx Received: from bigwig.baldwin.cx (HELO mail.baldwin.cx) (96.47.65.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 03 Jan 2018 19:05:53 +0000 Received: from ralph.baldwin.cx (astound-66-234-202-155.ca.astound.net [66.234.202.155]) by mail.baldwin.cx (Postfix) with ESMTPSA id 492F710A8C2; Wed, 3 Jan 2018 14:05:51 -0500 (EST) From: John Baldwin To: Simon Marchi Cc: gdb-patches@sourceware.org, binutils@sourceware.org Subject: Re: [PATCH 3/4] Support 'info proc' for native FreeBSD processes. Date: Wed, 03 Jan 2018 19:05:00 -0000 Message-ID: <137803623.10H6MxTIBr@ralph.baldwin.cx> User-Agent: KMail/4.14.10 (FreeBSD/11.1-STABLE; KDE/4.14.30; amd64; ; ) In-Reply-To: References: <20171222220513.54983-1-jhb@FreeBSD.org> <20171222220513.54983-4-jhb@FreeBSD.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-IsSubscribed: yes X-SW-Source: 2018-01/txt/msg00035.txt.bz2 On Tuesday, December 26, 2017 09:22:52 PM Simon Marchi wrote: > On 2017-12-22 05:05 PM, John Baldwin wrote: > > + else > > + { > > + pid = strtol (built_argv[0], NULL, 10); > > + } > > Unnecessary curly braces. Fixed. > > +#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. > > + > > + 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? > > @@ -461,23 +828,6 @@ show_fbsd_lwp_debug (struct ui_file *file, int from_tty, > > } > > > > #if defined(TDP_RFPPWAIT) || defined(HAVE_STRUCT_PTRACE_LWPINFO_PL_TDNAME) > > -/* Fetch the external variant of the kernel's internal process > > - structure for the process PID into KP. */ > > - > > -static void > > -fbsd_fetch_kinfo_proc (pid_t pid, struct kinfo_proc *kp) > > -{ > > - size_t len; > > - int mib[4]; > > - > > - len = sizeof *kp; > > - mib[0] = CTL_KERN; > > - mib[1] = KERN_PROC; > > - mib[2] = KERN_PROC_PID; > > - mib[3] = pid; > > - if (sysctl (mib, 4, kp, &len, NULL, 0) == -1) > > - perror_with_name (("sysctl")); > > -} > > #endif > > This leaves an empty #if/#endif. Should it be removed or moved with the function? Removed, good catch. -- John Baldwin