This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2 1/5] Support 'info proc' for FreeBSD process core dumps.
- From: Simon Marchi <simark at simark dot ca>
- To: John Baldwin <jhb at FreeBSD dot org>, gdb-patches at sourceware dot org
- Date: Thu, 4 Jan 2018 21:53:47 -0500
- Subject: Re: [PATCH v2 1/5] Support 'info proc' for FreeBSD process core dumps.
- Authentication-results: sourceware.org; auth=none
- References: <20180104014923.11899-1-jhb@FreeBSD.org> <20180104014923.11899-2-jhb@FreeBSD.org>
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