Re: [PATCH] arc: Add support for Linux coredump files

[Simon Marchi <simark@simark.ca>]

On 2020-09-28 10:08 a.m., Aktemur, Tankut Baris wrote:
> Hi Shahab,
>
> On Monday, September 28, 2020 3:47 PM, Shahab Vahedi wrote:
>> Hi Baris,
>>
>> On Thu, Sep 17, 2020 at 11:55:42AM +0000, Aktemur, Tankut Baris wrote:
>>>> On 2020-08-27 7:27 a.m., Shahab Vahedi via Gdb-patches wrote:
>>>> +bool
>>>> +arc_target::low_breakpoint_at (CORE_ADDR where)
>>>> +{
>>>> +  uint16_t insn;
>>>> +  uint16_t breakpoint = ntohs (TRAP_S_1_OPCODE);
>>>> +
>>>> +  the_target->read_memory (where, (gdb_byte *) &insn, TRAP_S_1_SIZE);
>>>
>>> Because 'the_target' is the same as 'this', you may want to simply call
>>> 'read_memory' without explicitly stating the receiver object.  This way,
>>> accessing the global variable could be avoided.
>>
>> Thank you very much for your input, but since it is not very obvious to
>> me where/when the "the_target" is initialized, I'd rather keep it this
>> way. Specially seeing that other targets (ARM and RISCV) doing the same.
>>
>>
>> Cheers,
>> Shahab
>
> Just for the record, there are similar uses in linux-aarch32-low.cc,
> linux-arm-low.cc, linux-low.cc, linux-sparc-low.cc, and win32-low.cc (I don't
> see it in linux-riscv-low.cc), and in all of these uses the context is a function
> (not a method) where the 'this' pointer is not available.
>
> Regards
> -Baris

FWIW, I'm leaning towards what Baris says, better use "this" than the
global variable.  And I even prefer to make it explicit,
"this->read_memory (...", to make it clear that we call a method and not
a free function.  But I don't want to start an endless discussion about
coding style :).

Simon