[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 3/4] secret: Properly handle @def after virSecretObjAdd in driver



On Tue, Jul 25, 2017 at 08:23:04AM -0400, John Ferlan wrote:
> 
> 
> On 07/25/2017 07:36 AM, Pavel Hrdina wrote:
> > On Fri, Jul 14, 2017 at 10:04:41AM -0400, John Ferlan wrote:
> >> Since the virSecretObjListAdd technically consumes @def on success,
> >> the secretDefineXML should set @def = NULL immediately and process
> >> the remaining calls using a new @objdef variable. We can use use
> >> VIR_STEAL_PTR since we know the Add function just stores @def in
> >> obj->def.
> >>
> >> This fixes a possible double free of @def if the code jumps to
> >> restore_backup: and calls virSecretObjListRemove without setting
> >> def = NULL. In this case, the subsequent call to DefFree would
> >> succeed and free @def; however, the call to EndAPI would also
> >> call DefFree because the Unref done would be the last one for
> >> the @obj meaning the obj->def would be used to call DefFree,
> >> but it's already been free'd because @def wasn't managed right
> >> within this error path.
> >>
> >> Signed-off-by: John Ferlan <jferlan redhat com>
> >> ---
> >>  src/secret/secret_driver.c | 19 ++++++++++---------
> >>  1 file changed, 10 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
> >> index 30124b4..77351d8 100644
> >> --- a/src/secret/secret_driver.c
> >> +++ b/src/secret/secret_driver.c
> >> @@ -210,6 +210,7 @@ secretDefineXML(virConnectPtr conn,
> >>  {
> >>      virSecretPtr ret = NULL;
> >>      virSecretObjPtr obj = NULL;
> >> +    virSecretDefPtr objdef;
> > 
> > s/objdef/objDef/
> 
> Why?  I've been using objdef in general and not the camel case one

Well, the naming convention is usually a camelCase or snake_case.
In that case other usages of objdef are not correct as well.  Yes in
this case it's easy to distinguish the two parts on the variable name,
but I still thing that camelCase is the preferred form.

Pavel

> 
> John
> > 
> > Reviewed-by: Pavel Hrdina <phrdina redhat com>
> > 

Attachment: signature.asc
Description: Digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]