Re: [PATCH v2] Implement the ability to set/unset environment variables to GDBserver when starting the inferior

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

On 2017-08-05 01:03, Sergio Durigan Junior wrote:
> So, I've been thinking about this implementation over the last few 
> days,
> but it's still a bit confuse to me.  My C++-foo is not so good as 
> yours,
> so maybe you can give me a hand.
> 
> From what I understood initially, your intention was to make a new 
> class
> that inherited from std::vector but overloaded/implemented methods to
> mimic what a std::set would do.  However, after reading a bit, it
> doesn't seem like a good idea to inherit from std::vector (or any std
> containers).  Which made me realize that maybe you are talking about
> creating a class that encapsulates a std::vector, without inheriting
> from it, and that provided the regular .push_back, .insert, .empty,
> etc., methods, but in an enhanced way in order to e.g. prevent the
> insert of duplicated elements, which is one of the things we miss from
> std::set.

Do you mean "... we miss from std::vector"?

I also read about inheriting STL containers.  The danger is that they 
don't have virtual destructors.  So let's say we have gdb::set_vector 
inheriting std::vector and try to delete a set_vector instance through a 
vector pointer, the set_vector destructor won't be called.  If 
set_vector is simply adding some methods to the interface (no data 
members, no user defined destructor), I don't know if this is really a 
problem.

But now that I think of it, if we want to make sure this object 
guarantees the properties of a set (like no duplicate elements), we 
would have to hide most of the vector interface and only expose a 
set-like interface.  Otherwise, it would be possible to call push_back 
and insert a duplicate element.  Given that, I'd lean towards creating a 
class that includes a vector and exposes a set-like interface, rather 
than inheriting.

> 
> Am I in the right direction here?  Also, I started to think...  I don't
> envision the user setting thousands of user-set environment variables,
> so *maybe* using std::set would be OK-ish for our use case scenario.  I
> don't know.  While I understand the concern about premature
> pessimization, I'm also not a fan of complicating the implementation
> just for a little bit of optimization either.
> 
> WDYT?

Actually, if we expected the user to set thousands of environment 
variables and needed it to be fast to set and unset variables, it would 
be good to use std::set because of the O(log(N)) 
lookups/insertions/removals, which matters more when you have a lot of 
elements.  But when you just have a few elements, the constant cost is 
more significant.  A vector-based set would have O(N) complexity for 
these operations (at least for insertions and removals, for lookups it 
depends if it is sorted), which would be bad if we had thousands of 
elements.  But since we expect to have just a few, it would likely be 
faster than std::set's constant cost.

But I agree with you that the important thing now is to get the 
algorithm/functionality right.  The typical use case is to have at the 
maximum a few inferiors, which may have at the maximum a few environment 
variables defined.  The defined environment variables are only used when 
doing a "run", which doesn't happen often.  So in the end, I don't think 
it would make a perceptible difference in memory usage or execution time 
to use an std::set.  If it helps to make the code significantly simpler, 
then I think it's worth it.  And we can always revisit it later by 
making a drop-in replacement for std::set.

Simon