Re: [PATCH v6] C++ify gdb/common/environ.c

[Pedro Alves <palves@redhat.com>]

On 06/19/2017 05:26 PM, Simon Marchi wrote:
> On 2017-06-19 17:44, Pedro Alves wrote:
>> If we take the "always push a NULL on construction" approach, and
>> we want moved-from gdb_environs to be valid, then yes.  Note how this
>> results in extra heap allocations when e.g., returning a
>> gdb_environ from functions by value, and makes std::vector<gdb_environ>
>> much less efficient when it decides it needs to reallocate/move
>> elements.  Representing the empty state with a cleared internal
>> vector would avoid this.
> 
> Given the move case, since the goal is to be efficient, then yeah I
> would agree
> that it would make sense to make a little bit of efforts to avoid
> allocating
> memory for an objects we are almost certainly throwing away.
> 
> But still, in order to leave environ objects in a valid state after a
> move and
> to pedantically comply with the STL spec which says that the vector is
> left in
> an unspecified state, shouldn't we do a .clear () on the moved-from
> vector after
> the move?

See accepted answer at:

 https://stackoverflow.com/questions/17730689/is-a-moved-from-vector-always-empty

So the only case where it'd be needed would be in op=, and iff the 
vectors had different allocators, which is not the case here.
So no, it's not necessary.  But I'd be fine with calling it.

> 
>> Note BTW, that we need to be careful with self-move leaving the
>> *this object in a valid state.
> 
> Should we just do
> 
> if (&other == this)
>   return *this;

Might not be necessary if without that the object ends up
valid anyway.  But what you wrote is a safe bet.

Thanks,
Pedro Alves