Re: [PATCH] disable ptrace BookE interface for PowerPC server processors

[Edjunior Barbosa Machado <emachado at linux dot vnet dot ibm dot com>]

On 05/31/2012 05:55 PM, Sergio Durigan Junior wrote:

> On Friday, May 25 2012, Edjunior Barbosa Machado wrote:
>> gdb/
>> 2012-25-05  Edjunior Machado  <emachado@linux.vnet.ibm.com>
>>
>> 	* ppc-linux-nat.c (have_ptrace_booke_interface): disable ptrace booke
>> 	interface for powerpc server processors if not available in
>> 	kernel.
> 
> I guess you could rewrite this to:
> 
>         * ppc-linux-nat.c (have_ptrace_booke_interface): Disable ptrace
>           BookE interface for PowerPC server processors if not available
>           in the Linux kernel.
> 
>> diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
>> index 34c40b4..9ffcf88 100644
>> --- a/gdb/ppc-linux-nat.c
>> +++ b/gdb/ppc-linux-nat.c
>> @@ -1421,17 +1421,18 @@ have_ptrace_booke_interface (void)
>>        /* Check for kernel support for BOOKE debug registers.  */
>>        if (ptrace (PPC_PTRACE_GETHWDBGINFO, tid, 0, &booke_debug_info) >= 0)
>>  	{
>> -	  have_ptrace_booke_interface = 1;
>> -	  max_slots_number = booke_debug_info.num_instruction_bps
>> -	    + booke_debug_info.num_data_bps
>> -	    + booke_debug_info.num_condition_regs;
>> -	}
>> -      else
>> -	{
>> -	  /* Old school interface and no BOOKE debug registers support.  */
>> -	  have_ptrace_booke_interface = 0;
>> -	  memset (&booke_debug_info, 0, sizeof (struct ppc_debug_info));
> 
> 
>> +	  if (booke_debug_info.features)
> 
> I'd rather use an explicit check like this:
> 
>           if (booke_debug_info.features != 0)
> 
> Also, I guess you could put a comment above this check and insert a
> brief explanation like you did in the top of this message, WDYT?
> 
> From what I remember when I hacked this interface, the patch is good
> (and somewhat trivial to me) :-).
> 
> Thanks,
> 


Thanks for the review, Sergio.

Based on your reply and the feedback from Mark, I've just committed the
following version of the patch.

Thanks,
-- 
Edjunior

gdb/
2012-05-31  Edjunior Machado  <emachado@linux.vnet.ibm.com>

	* ppc-linux-nat.c (have_ptrace_booke_interface): Disable ptrace
	BookE interface for PowerPC server processors if not available
	in the Linux Kernel.

diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index b9e0c85..45cdd73 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -1421,17 +1421,20 @@ have_ptrace_booke_interface (void)
       /* Check for kernel support for BOOKE debug registers.  */
       if (ptrace (PPC_PTRACE_GETHWDBGINFO, tid, 0, &booke_debug_info) >= 0)
 	{
-	  have_ptrace_booke_interface = 1;
-	  max_slots_number = booke_debug_info.num_instruction_bps
-	    + booke_debug_info.num_data_bps
-	    + booke_debug_info.num_condition_regs;
-	}
-      else
-	{
-	  /* Old school interface and no BOOKE debug registers support.  */
-	  have_ptrace_booke_interface = 0;
-	  memset (&booke_debug_info, 0, sizeof (struct ppc_debug_info));
+	  /* Check whether ptrace BOOKE interface is functional and
+	     provides any supported feature.  */
+	  if (booke_debug_info.features != 0)
+	    {
+	      have_ptrace_booke_interface = 1;
+	      max_slots_number = booke_debug_info.num_instruction_bps
+	        + booke_debug_info.num_data_bps
+	        + booke_debug_info.num_condition_regs;
+	      return have_ptrace_booke_interface;
+	    }
 	}
+      /* Old school interface and no BOOKE debug registers support.  */
+      have_ptrace_booke_interface = 0;
+      memset (&booke_debug_info, 0, sizeof (struct ppc_debug_info));
     }

   return have_ptrace_booke_interface;