Re: [PATCH v4] gdb: ADI support

Simon Marchi
2017-07-25T20:18:12Z

On 2017-07-25 21:19, Weimin Pan wrote:
> ----- Original Message -----
>> From: qiyaoltc@gmail.com
>> To: jose.marchesi@oracle.com
>> Cc: weimin.pan@oracle.com, gdb-patches@sourceware.org
>> Sent: Tuesday, July 25, 2017 9:44:55 AM GMT -08:00 US/Canada Pacific
>> Subject: Re: [PATCH v4] gdb: ADI support
>>
>> jose.marchesi@oracle.com (Jose E. Marchesi) writes:
>>
>> > General question: what's wrong with using a simple linked list of
>> > structs if that is the developer's preference, and/or it follows the
>> > current style of the surrounding code? (Not saying it is in this case,
>> > that's up to Weimin to say.)
>>
>> It makes the code a little bit unnecessarily complicated, IOW,
>> std::list
>> or std::forward_list can make it simpler. My preference is to use
>> standard c++ data structure rather than re-inventing it again.
>
> Obviously I missed your comment of using std::list but it's not clear
> to me why
> you think a simple linked list can be a little bit unnecessarily
> complicated.

First, as Yao said, using a home-made linked list is like re-inventing
the wheel. It is more complex to review/read code that plays with
pointers and pointers to pointers than one that uses standard APIs like
push_back or remove/remove_if. It's also less boilerplate code, so less
noise around the code that actually does what we want.

Also, for some time we have been moving slowly but surely towards RAII
for managing memory (and other resources). In my opinion, it has helped
a lot to clean up some parts of the code, and using C++ containers plays
a big role in that. I think it makes sense that we try to aim for that
in the new code, otherwise it's just more things we'll need to change in
the future.

>
> Given the fact that we need to backport this new feature to older
> gdb's, e.g.
> dev-toolset6, one question is do we need to resort to the linked list
> solution
> if the std list implementation is not available in older gdb versions?
> As of now,
> we have one diversion already - uiout->text ("\n") VS ui_out_text
> (uiout, "\n").

Are you talking about Red Hat devtoolset? If so, it seems like it
contains gdb 7.12. As far as I remember, it was possible to build 7.12
in both C and C++ mode. If you don't need to maintain the ability to
build as C, you should be able to backport a patch using std::list.

Thanks,

Simon