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@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