On 04/25/2017 09:18 AM, Pavel Hrdina wrote:
> On Mon, Apr 24, 2017 at 03:18:31PM -0400, John Ferlan wrote:
>> Rather than dereferencing obj->def->XXX or nwfilters->objs[i]->X
>> create local virNWFilterObjPtr and virNWFilterDefPtr variables.
>>
>> Future adjustments will be privatizing the object more, so this just
>> prepares the code for that reality.
>>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>> src/conf/virnwfilterobj.c | 80 +++++++++++++++++++++++++++---------------
>> src/nwfilter/nwfilter_driver.c | 33 ++++++++++-------
>> 2 files changed, 72 insertions(+), 41 deletions(-)
>>
>> diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
>> index cb697f9..3c6bdbb 100644
>> --- a/src/conf/virnwfilterobj.c
>> +++ b/src/conf/virnwfilterobj.c
>> @@ -37,11 +37,16 @@ VIR_LOG_INIT("conf.virnwfilterobj");
>> void
>> virNWFilterObjFree(virNWFilterObjPtr obj)
>> {
>> + virNWFilterDefPtr def;
>> + virNWFilterDefPtr newDef;
>> +
>> if (!obj)
>> return;
>> + def = obj->def;
>> + newDef = obj->newDef;
>>
>> - virNWFilterDefFree(obj->def);
>> - virNWFilterDefFree(obj->newDef);
>> + virNWFilterDefFree(def);
>> + virNWFilterDefFree(newDef);
>
> This was discussed in the secret cleanup series. In this case it just adds
> some lines to the code without any real benefit, so it's just a noise in this
> case. This change makes sense for functions where the *def* is used several
> times, but for those simple usages of def there is no point of having a
> separate variable.
>
> Now I know that some future patches may benefit from this change, however
> there is no guarantee that the patches will be accepted and pushed I think
> that you should wait with these changes for that future series.
>
> Pavel
>
I wouldn't be the first and I won't be the last to make/post changes to
code to help some future yet unposted series that has the some 'issue'
w/r/t a "guarantee" that it would accepted/pushed.
Like I pointed out in the secrets series - stopping progress here
because it's undesirable to have @def alone doesn't mean it's not
technically correct and the reality is the compiler will do whatever it
wants...
Again, like the secrets code I can agree if it's just "obj->def", I can
restore that, but once it's "obj->def->X", then having a "def" is more
desirable regardless if there's only one such reference in the code.
Pulling this patch from the entire series begins to impact (e.g. force
me down the git merge path) starting at patch 5. So it's very painful
to do.
John
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
