[RFC] remove target_acknowledge_created_inferior

[RFC] remove target_acknowledge_created_inferior

[Joel Brobecker]

This target_ops routine appears to be unused in practice, so this patch
removes it.  This is an RFC because I'm wondering whether we might want
to keep this anyway, if we know of at least one platform where this would
be necessary?

gdb/ChangeLog:

        * inf-child.c (inf_child_acknowledge_created_inferior): Delete.
        (inf_child_target): Do not set t->to_acknowledge_created_inferior.
        * inf-ptrace.c (inf_ptrace_create_inferior): Remove call to
        target_acknowledge_created_inferior.
        * inf-ttrace.c (inf_ttrace_him): Likewise.
        * target.h (struct target_ops): Delete to_acknowledge_created_inferior
        field.
        (target_acknowledge_created_inferior): Delete.
        * target.c (update_current_target): Remove setting of
        to_acknowledge_created_inferior field. Do default value for that
        field either.
        (debug_to_acknowledge_created_inferior): Delete.
        (setup_target_debug): Remove setting of
        current_target.to_acknowledge_created_inferior.

Tested on x86_64-linux.

---
 gdb/inf-child.c  |    8 --------
 gdb/inf-ptrace.c |    7 -------
 gdb/inf-ttrace.c |    7 -------
 gdb/target.c     |   14 --------------
 gdb/target.h     |    7 -------
 5 files changed, 0 insertions(+), 43 deletions(-)

diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index 72a18e4..ef652b8 100644
--- a/gdb/inf-child.c
+++ b/gdb/inf-child.c
@@ -88,13 +88,6 @@ inf_child_post_startup_inferior (ptid_t ptid)
 }
 
 static void
-inf_child_acknowledge_created_inferior (int pid)
-{
-  /* This version of Unix doesn't require a meaningful "acknowledge
-     created inferior" operation by a debugger.  */
-}
-
-static void
 inf_child_insert_fork_catchpoint (int pid)
 {
   /* This version of Unix doesn't support notification of fork
@@ -192,7 +185,6 @@ inf_child_target (void)
   t->to_terminal_ours = terminal_ours;
   t->to_terminal_info = child_terminal_info;
   t->to_post_startup_inferior = inf_child_post_startup_inferior;
-  t->to_acknowledge_created_inferior = inf_child_acknowledge_created_inferior;
   t->to_insert_fork_catchpoint = inf_child_insert_fork_catchpoint;
   t->to_remove_fork_catchpoint = inf_child_remove_fork_catchpoint;
   t->to_insert_vfork_catchpoint = inf_child_insert_vfork_catchpoint;
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index 440f2ed..d64c519 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -139,13 +139,6 @@ inf_ptrace_create_inferior (struct target_ops *ops,
   if (! ops_already_pushed)
     discard_cleanups (back_to);
 
-  /* On some targets, there must be some explicit synchronization
-     between the parent and child processes after the debugger
-     forks, and before the child execs the debuggee program.  This
-     call basically gives permission for the child to exec.  */
-
-  target_acknowledge_created_inferior (pid);
-
   /* START_INFERIOR_TRAPS_EXPECTED is defined in inferior.h, and will
      be 1 or 2 depending on whether we're starting without or with a
      shell.  */
diff --git a/gdb/inf-ttrace.c b/gdb/inf-ttrace.c
index aa0462f..dd1c4ba 100644
--- a/gdb/inf-ttrace.c
+++ b/gdb/inf-ttrace.c
@@ -625,13 +625,6 @@ inf_ttrace_him (struct target_ops *ops, int pid)
 
   push_target (ops);
 
-  /* On some targets, there must be some explicit synchronization
-     between the parent and child processes after the debugger forks,
-     and before the child execs the debuggee program.  This call
-     basically gives permission for the child to exec.  */
-
-  target_acknowledge_created_inferior (pid);
-
   /* START_INFERIOR_TRAPS_EXPECTED is defined in inferior.h, and will
      be 1 or 2 depending on whether we're starting without or with a
      shell.  */
diff --git a/gdb/target.c b/gdb/target.c
index 5984125..91a3841 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -624,7 +624,6 @@ update_current_target (void)
       INHERIT (to_lookup_symbol, t);
       /* Do no inherit to_create_inferior.  */
       INHERIT (to_post_startup_inferior, t);
-      INHERIT (to_acknowledge_created_inferior, t);
       INHERIT (to_insert_fork_catchpoint, t);
       INHERIT (to_remove_fork_catchpoint, t);
       INHERIT (to_insert_vfork_catchpoint, t);
@@ -778,9 +777,6 @@ update_current_target (void)
   de_fault (to_post_startup_inferior,
 	    (void (*) (ptid_t))
 	    target_ignore);
-  de_fault (to_acknowledge_created_inferior,
-	    (void (*) (int))
-	    target_ignore);
   de_fault (to_insert_fork_catchpoint,
 	    (void (*) (int))
 	    tcomplain);
@@ -3649,15 +3645,6 @@ debug_to_post_startup_inferior (ptid_t ptid)
 }
 
 static void
-debug_to_acknowledge_created_inferior (int pid)
-{
-  debug_target.to_acknowledge_created_inferior (pid);
-
-  fprintf_unfiltered (gdb_stdlog, "target_acknowledge_created_inferior (%d)\n",
-		      pid);
-}
-
-static void
 debug_to_insert_fork_catchpoint (int pid)
 {
   debug_target.to_insert_fork_catchpoint (pid);
@@ -3831,7 +3818,6 @@ setup_target_debug (void)
   current_target.to_load = debug_to_load;
   current_target.to_lookup_symbol = debug_to_lookup_symbol;
   current_target.to_post_startup_inferior = debug_to_post_startup_inferior;
-  current_target.to_acknowledge_created_inferior = debug_to_acknowledge_created_inferior;
   current_target.to_insert_fork_catchpoint = debug_to_insert_fork_catchpoint;
   current_target.to_remove_fork_catchpoint = debug_to_remove_fork_catchpoint;
   current_target.to_insert_vfork_catchpoint = debug_to_insert_vfork_catchpoint;
diff --git a/gdb/target.h b/gdb/target.h
index 7290d90..ea27b2c 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -467,7 +467,6 @@ struct target_ops
     void (*to_create_inferior) (struct target_ops *, 
 				char *, char *, char **, int);
     void (*to_post_startup_inferior) (ptid_t);
-    void (*to_acknowledge_created_inferior) (int);
     void (*to_insert_fork_catchpoint) (int);
     int (*to_remove_fork_catchpoint) (int);
     void (*to_insert_vfork_catchpoint) (int);
@@ -1026,12 +1025,6 @@ void target_create_inferior (char *exec_file, char *args,
 #define target_post_startup_inferior(ptid) \
      (*current_target.to_post_startup_inferior) (ptid)
 
-/* On some targets, the sequence of starting up an inferior requires
-   some synchronization between gdb and the new inferior process, PID.  */
-
-#define target_acknowledge_created_inferior(pid) \
-     (*current_target.to_acknowledge_created_inferior) (pid)
-
 /* On some targets, we can catch an inferior fork or vfork event when
    it occurs.  These functions insert/remove an already-created
    catchpoint for such events.  */
-- 
1.7.1

Re: [RFC] remove target_acknowledge_created_inferior

[Pedro Alves]

On Tuesday 28 December 2010 07:46:30, Joel Brobecker wrote:
> This target_ops routine appears to be unused in practice, so this patch
> removes it.  This is an RFC because I'm wondering whether we might want
> to keep this anyway, if we know of at least one platform where this would
> be necessary?

The details of target_create_inferior/fork_child changed enough since
this was needed, that I don't think it's necessary anywhere anymore.
A target can just call whatever it would place there after fork_inferior
returning.  I say just go for it.  Easy to add back if proven necessary.

> 
> gdb/ChangeLog:
> 
>         * inf-child.c (inf_child_acknowledge_created_inferior): Delete.
>         (inf_child_target): Do not set t->to_acknowledge_created_inferior.
>         * inf-ptrace.c (inf_ptrace_create_inferior): Remove call to
>         target_acknowledge_created_inferior.
>         * inf-ttrace.c (inf_ttrace_him): Likewise.
>         * target.h (struct target_ops): Delete to_acknowledge_created_inferior
>         field.
>         (target_acknowledge_created_inferior): Delete.
>         * target.c (update_current_target): Remove setting of
>         to_acknowledge_created_inferior field. Do default value for that
>         field either.
>         (debug_to_acknowledge_created_inferior): Delete.
>         (setup_target_debug): Remove setting of
>         current_target.to_acknowledge_created_inferior.
> 
> Tested on x86_64-linux.
> 
> ---
>  gdb/inf-child.c  |    8 --------
>  gdb/inf-ptrace.c |    7 -------
>  gdb/inf-ttrace.c |    7 -------
>  gdb/target.c     |   14 --------------
>  gdb/target.h     |    7 -------
>  5 files changed, 0 insertions(+), 43 deletions(-)
> 
> diff --git a/gdb/inf-child.c b/gdb/inf-child.c
> index 72a18e4..ef652b8 100644
> --- a/gdb/inf-child.c
> +++ b/gdb/inf-child.c
> @@ -88,13 +88,6 @@ inf_child_post_startup_inferior (ptid_t ptid)
>  }
>  
>  static void
> -inf_child_acknowledge_created_inferior (int pid)
> -{
> -  /* This version of Unix doesn't require a meaningful "acknowledge
> -     created inferior" operation by a debugger.  */
> -}
> -
> -static void
>  inf_child_insert_fork_catchpoint (int pid)
>  {
>    /* This version of Unix doesn't support notification of fork
> @@ -192,7 +185,6 @@ inf_child_target (void)
>    t->to_terminal_ours = terminal_ours;
>    t->to_terminal_info = child_terminal_info;
>    t->to_post_startup_inferior = inf_child_post_startup_inferior;
> -  t->to_acknowledge_created_inferior = inf_child_acknowledge_created_inferior;
>    t->to_insert_fork_catchpoint = inf_child_insert_fork_catchpoint;
>    t->to_remove_fork_catchpoint = inf_child_remove_fork_catchpoint;
>    t->to_insert_vfork_catchpoint = inf_child_insert_vfork_catchpoint;
> diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
> index 440f2ed..d64c519 100644
> --- a/gdb/inf-ptrace.c
> +++ b/gdb/inf-ptrace.c
> @@ -139,13 +139,6 @@ inf_ptrace_create_inferior (struct target_ops *ops,
>    if (! ops_already_pushed)
>      discard_cleanups (back_to);
>  
> -  /* On some targets, there must be some explicit synchronization
> -     between the parent and child processes after the debugger
> -     forks, and before the child execs the debuggee program.  This
> -     call basically gives permission for the child to exec.  */
> -
> -  target_acknowledge_created_inferior (pid);
> -
>    /* START_INFERIOR_TRAPS_EXPECTED is defined in inferior.h, and will
>       be 1 or 2 depending on whether we're starting without or with a
>       shell.  */
> diff --git a/gdb/inf-ttrace.c b/gdb/inf-ttrace.c
> index aa0462f..dd1c4ba 100644
> --- a/gdb/inf-ttrace.c
> +++ b/gdb/inf-ttrace.c
> @@ -625,13 +625,6 @@ inf_ttrace_him (struct target_ops *ops, int pid)
>  
>    push_target (ops);
>  
> -  /* On some targets, there must be some explicit synchronization
> -     between the parent and child processes after the debugger forks,
> -     and before the child execs the debuggee program.  This call
> -     basically gives permission for the child to exec.  */
> -
> -  target_acknowledge_created_inferior (pid);
> -
>    /* START_INFERIOR_TRAPS_EXPECTED is defined in inferior.h, and will
>       be 1 or 2 depending on whether we're starting without or with a
>       shell.  */
> diff --git a/gdb/target.c b/gdb/target.c
> index 5984125..91a3841 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -624,7 +624,6 @@ update_current_target (void)
>        INHERIT (to_lookup_symbol, t);
>        /* Do no inherit to_create_inferior.  */
>        INHERIT (to_post_startup_inferior, t);
> -      INHERIT (to_acknowledge_created_inferior, t);
>        INHERIT (to_insert_fork_catchpoint, t);
>        INHERIT (to_remove_fork_catchpoint, t);
>        INHERIT (to_insert_vfork_catchpoint, t);
> @@ -778,9 +777,6 @@ update_current_target (void)
>    de_fault (to_post_startup_inferior,
>  	    (void (*) (ptid_t))
>  	    target_ignore);
> -  de_fault (to_acknowledge_created_inferior,
> -	    (void (*) (int))
> -	    target_ignore);
>    de_fault (to_insert_fork_catchpoint,
>  	    (void (*) (int))
>  	    tcomplain);
> @@ -3649,15 +3645,6 @@ debug_to_post_startup_inferior (ptid_t ptid)
>  }
>  
>  static void
> -debug_to_acknowledge_created_inferior (int pid)
> -{
> -  debug_target.to_acknowledge_created_inferior (pid);
> -
> -  fprintf_unfiltered (gdb_stdlog, "target_acknowledge_created_inferior (%d)\n",
> -		      pid);
> -}
> -
> -static void
>  debug_to_insert_fork_catchpoint (int pid)
>  {
>    debug_target.to_insert_fork_catchpoint (pid);
> @@ -3831,7 +3818,6 @@ setup_target_debug (void)
>    current_target.to_load = debug_to_load;
>    current_target.to_lookup_symbol = debug_to_lookup_symbol;
>    current_target.to_post_startup_inferior = debug_to_post_startup_inferior;
> -  current_target.to_acknowledge_created_inferior = debug_to_acknowledge_created_inferior;
>    current_target.to_insert_fork_catchpoint = debug_to_insert_fork_catchpoint;
>    current_target.to_remove_fork_catchpoint = debug_to_remove_fork_catchpoint;
>    current_target.to_insert_vfork_catchpoint = debug_to_insert_vfork_catchpoint;
> diff --git a/gdb/target.h b/gdb/target.h
> index 7290d90..ea27b2c 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -467,7 +467,6 @@ struct target_ops
>      void (*to_create_inferior) (struct target_ops *, 
>  				char *, char *, char **, int);
>      void (*to_post_startup_inferior) (ptid_t);
> -    void (*to_acknowledge_created_inferior) (int);
>      void (*to_insert_fork_catchpoint) (int);
>      int (*to_remove_fork_catchpoint) (int);
>      void (*to_insert_vfork_catchpoint) (int);
> @@ -1026,12 +1025,6 @@ void target_create_inferior (char *exec_file, char *args,
>  #define target_post_startup_inferior(ptid) \
>       (*current_target.to_post_startup_inferior) (ptid)
>  
> -/* On some targets, the sequence of starting up an inferior requires
> -   some synchronization between gdb and the new inferior process, PID.  */
> -
> -#define target_acknowledge_created_inferior(pid) \
> -     (*current_target.to_acknowledge_created_inferior) (pid)
> -
>  /* On some targets, we can catch an inferior fork or vfork event when
>     it occurs.  These functions insert/remove an already-created
>     catchpoint for such events.  */
> 


-- 
Pedro Alves

Re: [RFC] remove target_acknowledge_created_inferior

[Joel Brobecker]

> The details of target_create_inferior/fork_child changed enough since
> this was needed, that I don't think it's necessary anywhere anymore.
> A target can just call whatever it would place there after fork_inferior
> returning.  I say just go for it.  Easy to add back if proven necessary.

Thanks! This makes sense to me too, so I checked the patch in.

-- 
Joel