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@xxxxxxxxxx> >>> --- >>> 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
