This is the mail archive of the
mailing list for the GDB project.
Re: [RFA, 1 of 3] save/restore process record, part 1 (exec_entry)
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Michael Snyder <msnyder at vmware dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>, Hui Zhu <teawater at gmail dot com>
- Date: Fri, 16 Oct 2009 21:03:52 -0700
- Subject: Re: [RFA, 1 of 3] save/restore process record, part 1 (exec_entry)
- References: <4AD91C32.firstname.lastname@example.org>
> 2009-10-16 Hui Zhu <email@example.com>
> Michael Snyder <firstname.lastname@example.org>
> * record.c (record_exec_entry): New function. Emulate one
> instruction, forward or backward. Abstracted from record_wait.
> (record_wait) Call record_exec_entry.
I can personnally only comment on details, since I don't know much
about process record. Not sure who from the Global Maintainers actually
know much about it except you, Michael :).
> +static inline void
> +record_exec_entry (struct regcache *regcache, struct gdbarch *gdbarch,
> + struct record_entry *entry)
We're really pushing for having all functions properly documented.
Can you add a comment explaining that this function does? The function
name makes it more or less obvious, I guess, but I'd personally rather
have a consistent (mindless) approach of documenting everything rather
than having to judge on a case-by-case basis.
Also, I can't help but wonder about the use of "inline" in this case.
I'm always reluctant to use this sort of feature until I can be proven
that this helps performance. Since this is a static function, I would
imagine that the compiler would have more knowledge of whether the
function should be inlined or not? Or is that too naive?
Other than than, seems like a pretty mechanical change...