[lvm-devel] [PATCH 1/3] Fix mem

[Zdenek Kabelac]

Dne 26.3.2011 10:59, Milan Broz napsal(a):
> On 03/25/2011 10:57 PM, Zdenek Kabelac wrote:
> 
>> -		unlock_and_free_vg(cmd, vg_from, vg_name_from);
>> -		unlock_and_free_vg(cmd, vg_to, vg_name_to);
>> +		unlock_vg(cmd, vg_name_from);
>> +		unlock_vg(cmd, vg_name_to);
>> +		free_vg(vg_to);
>> +		free_vg(vg_from);
> 
> I guess there is still request to do proper deep copy...
> 
> Anyway, this is so simple and obvious for now -> ACK


In fact there is even simpler version - to always unconditionally unlock and
free VGs in this order:

--
unlock_and_free_vg(cmd, vg_to, vg_name_to);
unlock_and_free_vg(cmd, vg_from, vg_name_from);
--

This simplifies and shortens the code a bit ;). 'vg_to' is always released
before 'vg_from' - if we make this a documented rule in the source code - we
currently avoid the need of deep copy (It would quite complicate our code, to
add such 'deep' copy code to every target - as we are not in C++ - it's quite
ugly task to do).  As long as we only move object pointers from 'vg_from' to
'vg_to' it's safe - and we may even avoid partial 'deep' copy of some object
there - if we can count with the right free_vg() order.
(Also implementing of deep-copy only for very occasionally used vgmerge and
vgsplit currently sound like a bit to complex).

AFAIK - for deadlock-less locking - only the locking order is important,
unlocking could happen in any order as long as all locks are always released
before the next usage - so current unlock ordering is not needed.

Zdenek