Re: [PATCH 1/5] Support multiple breakpoint types per target in GDBServer.

[Yao Qi <qiyaoltc@gmail.com>]

Antoine Tremblay <antoine.tremblay@ericsson.com> writes:

>> PC can't be NULL after your patch #2.  You can remove the second
>> sentence in this patch or patch #2.
>>
> I think you mean after patch #3 ?
>
> But it can still be NULL see in #3 :
>
> /* Default if no pc is set to arm breakpoint.  */
> +  if (pcptr == NULL)
> +    {
> +      *lenptr = arm_breakpoint_len;
> +      return (unsigned char *) &arm_breakpoint;
> +    }

I meant this change below in patch #2,

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index bb08761..06387a0 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -7069,16 +7069,10 @@ void
 initialize_low (void)
 {
   struct sigaction sigchld_action;
-  int breakpoint_len = 0;
-  const unsigned char *breakpoint = NULL;
 
   memset (&sigchld_action, 0, sizeof (sigchld_action));
   set_target_ops (&linux_target_ops);
 
-  breakpoint = the_target->breakpoint_from_pc (NULL, &breakpoint_len);
-
-  set_breakpoint_data (breakpoint,
-		       breakpoint_len);
   linux_init_signals ();
   linux_ptrace_init_warnings ();

We only pass NULL to breakpoint_from_pc here, and we remove it from
patch #2.  That is why I suggest that PCPTR can't be NULL.

[I am still reviewing this patch series.  I get something I can't
explain after I enable thread event breakpoint in order to exercise
your patches.  I'll send out my comments once I understand the them
fully.]

-- 
Yao (齐尧)