This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Introduce utility function find_inferior_ptid
- From: Joel Brobecker <brobecker at adacore dot com>
- To: simon dot marchi at ericsson dot com
- Cc: gdb-patches at sourceware dot org, simon dot marchi at polymtl dot ca
- Date: Sat, 13 Dec 2014 09:40:58 -0500
- Subject: Re: [PATCH] Introduce utility function find_inferior_ptid
- Authentication-results: sourceware.org; auth=none
- References: <1418412193-6259-1-git-send-email-simon dot marchi at ericsson dot com>
> This patch introduces find_inferior_ptid to replace the common idiom
>
> find_inferior_pid (ptid_get_pid (...));
>
> It replaces all the instances of that idiom that I found with the new
> function.
>
> The change was mostly mechanical and built-tested.
>
> gdb/ChangeLog:
>
> * inferior.c (find_inferior_ptid): New function.
> * inferior.h (find_inferior_ptid): New declaration.
> * ada-tasks.c (ada_get_task_number): Use find_inferior_ptid.
> * corelow.c (core_pid_to_str): Same.
> * darwin-nat.c (darwin_resume): Same.
> * infrun.c (fetch_inferior_event): Same.
> (get_inferior_stop_soon): Same.
> (handle_inferior_event): Same.
> (handle_signal_stop): Same.
> * linux-nat.c (resume_lwp): Same.
> (stop_wait_callback): Same.
> * mi/mi-interp.c (mi_new_thread): Same.
> (mi_thread_exit): Same.
> * proc-service.c (ps_pglobal_lookup): Same.
> * record-btrace.c (record_btrace_step_thread): Same.
> * remote-sim.c (gdbsim_close_inferior): Same.
> (gdbsim_resume): Same.
> (gdbsim_stop): Same.
> * sol2-tdep.c (sol2_core_pid_to_str): Same.
> * target.c (memory_xfer_partial_1): Same.
> (default_thread_address_space): Same.
> * thread.c (thread_change_ptid): Same.
> (switch_to_thread): Same.
> (do_restore_current_thread_cleanup): Same.
Overall, this looks reasonable. A couple of comments below.
Also, while at it, I would prefer if you tested the change.
With the number of mechanical changes, it's really easy to
let one bad change slip by, no matter how careful we try
to be; testing does not cost much so let's add that as well.
>
> +struct inferior *
> +find_inferior_ptid (ptid_t ptid) {
> + return find_inferior_ptid (ptid);
> +}
This function needs an introductory commend ("/* See inferior.h */").
This is something we do systematically now, even if the function's
documentation is present elsewhere. The one-line comment allows us
to know that there is a command, in incidentally where it is.
Also, the opening curly brace needs to be on the next line. Therefore:
/* See inferior.h. */
struct inferior *
find_inferior_ptid (ptid_t ptid)
{
return find_inferior_ptid (ptid);
}
> +/* Search function to lookup an inferior whose pid is equal to 'ptid.pid'. */
> +extern struct inferior * find_inferior_ptid (ptid_t ptid);
No space after the '*'.
Pre-approved with those changes and after testing confirmed.
Thank you,
--
Joel