From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 52155 invoked by alias); 5 Jan 2018 02:54:01 -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 48153 invoked by uid 89); 5 Jan 2018 02:53:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=H*Ad:D*ca, HContent-Transfer-Encoding:8bit 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; Fri, 05 Jan 2018 02:53:49 +0000 Received: from [10.0.0.11] (192-222-251-162.qc.cable.ebox.net [192.222.251.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id AA4D61E02D; Thu, 4 Jan 2018 21:53:47 -0500 (EST) Subject: Re: [PATCH v2 1/5] Support 'info proc' for FreeBSD process core dumps. To: John Baldwin , gdb-patches@sourceware.org References: <20180104014923.11899-1-jhb@FreeBSD.org> <20180104014923.11899-2-jhb@FreeBSD.org> From: Simon Marchi Message-ID: <8ce6ffcf-cf4c-9855-5d37-4d8db4e33803@simark.ca> Date: Fri, 05 Jan 2018 02:54:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <20180104014923.11899-2-jhb@FreeBSD.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2018-01/txt/msg00062.txt.bz2 Hi John, I have two little comments, but otherwise it LGTM. On 2018-01-03 08:49 PM, John Baldwin wrote: > +/* Implement "info proc status" for a corefile. */ > + > +static void > +fbsd_core_info_proc_status (struct gdbarch *gdbarch) > +{ > + const struct kinfo_proc_layout *kp; > + asection *section; > + const char *state; > + unsigned char *descdata, *descend; I get this: /home/simark/src/binutils-gdb/gdb/fbsd-tdep.c: In function ‘void fbsd_core_info_proc_status(gdbarch*)’: /home/simark/src/binutils-gdb/gdb/fbsd-tdep.c:791:29: error: variable ‘descend’ set but not used [-Werror=unused-but-set-variable] unsigned char *descdata, *descend; ^~~~~~~ > + int addr_bit, long_bit; > + size_t note_size; > + ULONGEST value; > + LONGEST sec; > + > + section = bfd_get_section_by_name (core_bfd, ".note.freebsdcore.proc"); > + if (section == NULL) > + { > + warning (_("unable to find process info in core file")); > + return; > + } > + > + addr_bit = gdbarch_addr_bit (gdbarch); > + if (addr_bit == 64) > + kp = &kinfo_proc_layout_64; > + else if (bfd_get_arch (core_bfd) == bfd_arch_i386) > + kp = &kinfo_proc_layout_i386; > + else > + kp = &kinfo_proc_layout_32; > + > + note_size = bfd_get_section_size (section); > + if (note_size < 4 + kp->ki_rusage_ch + kp->ru_majflt + addr_bit) What's the rationale behind that computation? The field ru_majflt in kp->ki_rusage_ch is the furthest field we'll actually need in this function? And addr_bit is the size of that field? And 4 is because there's a field at the beginning containing the size of the structure that comes after? Maybe a small comment would be good to remove the magic. Thanks, Simon