On 07/13/2017 10:41 AM, Michal Privoznik wrote:
> On 06/02/2017 12:25 PM, John Ferlan wrote:
>> If the virNWFilterSaveConfig in virNWFilterObjListLoadConfig, then jumping
>
> s/,/ fails,/
>
>> to the error label would free the @def owned by the object, but the object
>> is still on the list.
>>
>> Fix this by following proper procedure to clear @def and create a new
>> variable @objdef to handle the object's def after successful assignment.
>>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>> src/conf/virnwfilterobj.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
>> index 0027d45..3fb6046 100644
>> --- a/src/conf/virnwfilterobj.c
>> +++ b/src/conf/virnwfilterobj.c
>> @@ -485,6 +485,7 @@ virNWFilterObjListLoadConfig(virNWFilterObjListPtr nwfilters,
>> {
>> virNWFilterDefPtr def = NULL;
>> virNWFilterObjPtr obj;
>> + virNWFilterDefPtr objdef;
>> char *configFile = NULL;
>>
>> if (!(configFile = virFileBuildPath(configDir, name, ".xml")))
>> @@ -503,10 +504,12 @@ virNWFilterObjListLoadConfig(virNWFilterObjListPtr nwfilters,
>>
>> if (!(obj = virNWFilterObjListAssignDef(nwfilters, def)))
>> goto error;
>> + def = NULL;
>> + objdef = obj->def;
>>
>> /* We generated a UUID, make it permanent by saving the config to disk */
>> - if (!def->uuid_specified &&
>> - virNWFilterSaveConfig(configDir, def) < 0)
>> + if (!objdef->uuid_specified &&
>> + virNWFilterSaveConfig(configDir, objdef) < 0)
>> goto error;
>
> This @objdef variable looks redundant to me. Can't we access obj->def
> directly? Esp. since you're introducing a variable just for a two times
> use. Then again, we often use obj->def->... when it comes to domain
> objects, why not here?
>
If obj->def ever gets "consumed" by the virObject code, then obj->def->x
will fail miserably. That was the original design goal. I've since had
to scale back. I guess I could do "obj->def->uuid_specified" as it
doesn't really matter until the day obj->def-> is no longer accessible.
As a preference - I like the extra local variable. In the long run the
compiler will do away with it, but for me it just reads better that way.
The deeper one gets into a->b->c->d[->e...] the more insane it gets.
Forcing one level of indirection is just more readable to me.
John
> ACK if you drop the @objdef variable.
>
> Michal
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
