Re: [libvirt] [PATCH 6/6] conf: Use consistent function name prefixes for virnwfilterobj
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(a)redhat.com>
> ---
> 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