Re: [RFA, 1 of 3] save/restore process record, part 1 (exec_entry)

[Joel Brobecker <brobecker at adacore dot com>]

> 2009-10-16  Hui Zhu  <teawater@gmail.com>
> 	    Michael Snyder  <msnyder@vmware.com>
> 
> 	* 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...

-- 
Joel