Re: [PATCH v3 06/10] Replace breakpoint_reinsert_addr by get_next_pcs operation in GDBServer.

[Antoine Tremblay <antoine.tremblay@ericsson.com>]

On 11/26/2015 05:50 AM, Pedro Alves wrote:
> On 11/23/2015 02:12 PM, Antoine Tremblay wrote:
>> In this v3:
>>
>>   * Changed get_next_pcs signature.
>>   * Remove common/get-next-pcs.h file.
>>   * CORE_ADDR is now in linux-low.h temporarely.
>>
>> ---
>>
>> This patch in preparation for software single step support on ARM. It refactors
>> breakpoint_reinsert_addr into get_next_pcs so that multiple location can be
>> returned.
>>
>> When software single stepping there can be multiple possible next addresses
>> because we're stepping over a conditional branch instruction for example.
>>
>> The operation get_next_pcs handles that by returning a vector of all the
>> possible next addresses.
>>
>> Software breakpoints are installed at each location returned.
>>
>
>
> Thanks for doing this.  This interface does look cleaner to me, and a
> more direct mapping C++ (so easier to clean up in the future).
>

Indeed thanks for the idea!

>>
>> +/* Install breakpoints for software single stepping.  */
>> +
>> +static void
>> +install_software_single_step_breakpoints (struct lwp_info *lwp)
>> +{
>> +  int i;
>> +  CORE_ADDR pc;
>> +  struct regcache *regcache = get_thread_regcache (current_thread, 1);
>> +  VEC (CORE_ADDR) *next_pcs = NULL;
>
> I think this should be freed with a cleanup.
>
>> +
>> +  pc = get_pc (lwp);
>> +  next_pcs = (*the_low_target.get_next_pcs) (pc, regcache);
>> +
>> +  for (i = 0; VEC_iterate (CORE_ADDR, next_pcs, i, pc); ++i)
>> +    {
>> +      set_reinsert_breakpoint (pc);
>> +    }
>
> Single-line statements don't get braces.
>

Done, learning the style :)

Thank you,
Antoine