[libvirt] [PATCH] conf: Actually make virDomainChrSourceDef an object

Michal Privoznik mprivozn at redhat.com
Thu Apr 12 11:27:41 UTC 2018


On 04/12/2018 01:08 PM, John Ferlan wrote:
> 
> 
> On 04/12/2018 04:44 AM, Erik Skultety wrote:
>> On Thu, Apr 12, 2018 at 09:14:50AM +0200, Michal Privoznik wrote:
>>> In 2ada9ef1465f we've tried to turn virDomainChrSourceDef into
>>> virObject. Well, this requires 'virObject' member to be stored on
>>> the first position of the struct. This adjustment is missing in
>>> the original commit leading to all sorts of funny memleaks and
>>> data corruptions.
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>>> ---
>>>  src/conf/domain_conf.h | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
> 
> Doh!  Thanks....  I guess not only do we need a way to detect that we're
> using VIR_ALLOC on an object (as Eric pointed out in my original
> patches), but we really should detect when we use virObjectNew without
> the virObject parent. For some reason, I have a recollection of altering
> changes during my meanderings through virObject code in order to point
> out more directly some misuses, but it was rejected. Although I cannot
> recall if this particular instance of not having virObject parent was
> addressed...

I don't think it was addressed there. And honestly, I don't know how to
check for this in some automated way. Compiler is not going to help -
sure we can have very primitive check like sizeof(virSomeStruct) >=
sizeof(virObject), but that will fail only for very small structs (which
is not the case here). Also, writing script that would check for this is
going to end up in a lot of pain IMO. The only thing I can think of is
review.

Michal




More information about the libvir-list mailing list