On 03/07/2017 05:05 AM, Michal Privoznik wrote:
> On 03/06/2017 05:36 PM, John Ferlan wrote:
>> Use "virNWFilterObj" as a prefix for any external API in virnwfilterobj
>>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>> src/conf/virnwfilterobj.c | 34 +++++++++++++++++-----------------
>> src/conf/virnwfilterobj.h | 6 +++---
>> src/libvirt_private.syms | 4 ++--
>> src/nwfilter/nwfilter_driver.c | 8 +++-----
>> 4 files changed, 25 insertions(+), 27 deletions(-)
>>
>> diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
>> index 0d81912..7e13afb 100644
>> --- a/src/conf/virnwfilterobj.c
>> +++ b/src/conf/virnwfilterobj.c
>> @@ -117,9 +117,9 @@ virNWFilterObjFindByName(virNWFilterObjListPtr nwfilters,
>>
>>
>> static int
>> -_virNWFilterDefLoopDetect(virNWFilterObjListPtr nwfilters,
>> - virNWFilterDefPtr def,
>> - const char *filtername)
>> +_virNWFilterObjDefLoopDetect(virNWFilterObjListPtr nwfilters,
>> + virNWFilterDefPtr def,
>> + const char *filtername)
>
> Ouch. This is even worse than virFunction() + virFunctionInternal()
> pair. Moreover, I don't think that we need virNWFilterDefLoopDetect at
> all. I mean, this one starting with _ can take its place. The only
> difference is that while virNWFilterDefLoopDetect() takes just two
> arguments _virNWFilterDefLoopDetect() takes three.
Well _virNWFilterDefLoopDetect also calls itself recursively - it's a
rather nasty and ugly process from what I recall when I looked at it
while doing the RFC changes.
>
> But I guess that can be saved either for a follow up patch or one that's
> inserted before this one.
>
Ironically the other series did handle this... There's only one function
since lookup is handled differently.
I think I originally took the route of just changing the name of this
recursive function; however, I nixed that idea when the whole use a vir
prefix function discussion started.
I could change the name to be virNWFilterDefLoopDetectInternal in a
followup patch, but that'd perhaps only be a band-aid until I was able
to get the RFC changes into more robust patches. I can do a followup,
but it'd only be a name change...
John
>> {
>> int rc = 0;
>> size_t i;
>> @@ -141,8 +141,8 @@ _virNWFilterDefLoopDetect(virNWFilterObjListPtr nwfilters,
>> obj = virNWFilterObjFindByName(nwfilters,
>> entry->include->filterref);
>> if (obj) {
>> - rc = _virNWFilterDefLoopDetect(nwfilters,
>> - obj->def, filtername);
>> + rc = _virNWFilterObjDefLoopDetect(nwfilters,
>> + obj->def, filtername);
>>
>> virNWFilterObjUnlock(obj);
>> if (rc < 0)
>
> Michal
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
