Re: [PATCH v4] gdb: ADI support

[Simon Marchi <simon.marchi@polymtl.ca>]

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