Re: [PATCH 05/11 v5] Add target/target.h

[Pedro Alves <palves@redhat.com>]

On 08/01/2014 11:19 AM, Gary Benson wrote:

> diff --git a/gdb/target.h b/gdb/target.h
> index 4d91b6b..1a10744 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -58,6 +58,7 @@ struct dcache_struct;
>     it goes into the file stratum, which is always below the process
>     stratum.  */
>  
> +#include "target/target.h"
>  #include "target/resume.h"
>  #include "target/wait.h"
>  #include "target/waitstatus.h"
> @@ -1206,31 +1207,6 @@ extern void target_detach (const char *, int);
>  
>  extern void target_disconnect (const char *, int);
>  
> -/* Resume execution of the target process PTID (or a group of
> -   threads).  STEP says whether to single-step or to run free; SIGGNAL
> -   is the signal to be given to the target, or GDB_SIGNAL_0 for no
> -   signal.  The caller may not pass GDB_SIGNAL_DEFAULT.  A specific
> -   PTID means `step/resume only this process id'.  A wildcard PTID
> -   (all threads, or all threads of process) means `step/resume
> -   INFERIOR_PTID, and let other threads (for which the wildcard PTID
> -   matches) resume with their 'thread->suspend.stop_signal' signal
> -   (usually GDB_SIGNAL_0) if it is in "pass" state, or with no signal
> -   if in "no pass" state.  */
> -
> -extern void target_resume (ptid_t ptid, int step, enum gdb_signal signal);
> -
> -/* Wait for process pid to do something.  PTID = -1 to wait for any
> -   pid to do something.  Return pid of child, or -1 in case of error;
> -   store status through argument pointer STATUS.  Note that it is
> -   _NOT_ OK to throw_exception() out of target_wait() without popping
> -   the debugging target from the stack; GDB isn't prepared to get back
> -   to the prompt with a debugging target but without the frame cache,
> -   stop_pc, etc., set up.  OPTIONS is a bitwise OR of TARGET_W*
> -   options.  */
> -
> -extern ptid_t target_wait (ptid_t ptid, struct target_waitstatus *status,
> -			   int options);
> -
>  /* Fetch at least register REGNO, or all regs if regno == -1.  No result.  */
>  
>  extern void target_fetch_registers (struct regcache *regcache, int regno);
> @@ -1294,9 +1270,6 @@ int target_supports_disable_randomization (void);
>  
>  extern int target_read_string (CORE_ADDR, char **, int, int *);
>  
> -extern int target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
> -			       ssize_t len);
> -
>  extern int target_read_raw_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
>  				   ssize_t len);
>  
> @@ -1304,9 +1277,6 @@ extern int target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len);
>  
>  extern int target_read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len);
>  
> -extern int target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr,
> -				ssize_t len);
> -
>  extern int target_write_raw_memory (CORE_ADDR memaddr, const gdb_byte *myaddr,
>  				    ssize_t len);
>  
> @@ -1583,12 +1553,6 @@ extern int target_thread_alive (ptid_t ptid);
>  
>  extern void target_find_new_threads (void);
>  
> -/* Make target stop in a continuable fashion.  (For instance, under
> -   Unix, this should act like SIGSTOP).  This function is normally
> -   used by GUIs to implement a stop button.  */
> -
> -extern void target_stop (ptid_t ptid);
> -
>  /* Send the specified COMMAND to the target's monitor
>     (shell,interpreter) for execution.  The result of the query is
>     placed in OUTBUF.  */

I worry about the fragmentation that moving pieces of target
interfaces to target/target.h generates ends up confusing
people.  Did you consider leaving a marker in place, like:

 -extern ptid_t target_wait (ptid_t ptid, struct target_waitstatus *status,
 -			   int options);
 + /* For target_wait see target/target.h.  */

 -extern int target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr,
 -				ssize_t len);
 + /* For target_write_memory see target/target.h.  */

?

Thanks,
Pedro Alves