[libvirt] [PATCH v2 03/20] virlog: Introduce virLogFilterNew

[John Ferlan]

On 08/18/2016 07:47 AM, Erik Skultety wrote:
> This method allocated a new filter object which it then returns back to caller.

s/allocated/allocates

> 
> Signed-off-by: Erik Skultety <eskultet at redhat.com>
> ---
>  src/libvirt_private.syms |  2 ++
>  src/util/virlog.c        | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  src/util/virlog.h        |  3 +++
>  3 files changed, 51 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index b5cee5f..088f9f3 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1845,6 +1845,8 @@ virLockSpaceReleaseResourcesForOwner;
>  virLogDefineFilter;
>  virLogDefineOutput;
>  virLogFilterFree;
> +virLogFilterListFree;

^^  This is unrelated (so far) it seems...

> +virLogFilterNew;
>  virLogGetDefaultPriority;
>  virLogGetFilters;
>  virLogGetNbFilters;
> diff --git a/src/util/virlog.c b/src/util/virlog.c
> index 91c63a1..e4dc84b 100644
> --- a/src/util/virlog.c
> +++ b/src/util/virlog.c
> @@ -1604,3 +1604,49 @@ virLogOutputNew(virLogOutputFunc f,
>  
>      return ret;
>  }
> +
> +
> +/**
> + * virLogFilterNew:
> + * @match: the pattern to match
> + * @priority: the priority to give to messages matching the pattern
> + * @flags: extra flags, see virLogFilterFlags enum
> + *
> + * Allocates and returns a new log filter object. The object has to be later
> + * defined, so that the pattern will be taken into account when executing the
> + * log filters (to select or reject a particular message) on messages.
> + *
> + * The filter defines a rules that will apply only to messages matching
> + * the pattern (currently if @match is a substring of the message category)
> + *
> + * Returns a reference to a newly created filter that needs to be defined using
> + * virLogDefineFilters, or NULL in case of an error.
> + */
> +virLogFilterPtr
> +virLogFilterNew(const char *match,
> +                virLogPriority priority,
> +                unsigned int flags)
> +{
> +    virLogFilterPtr ret = NULL;
> +    char *mdup = NULL;
> +
> +    virCheckFlags(VIR_LOG_STACK_TRACE, NULL);
> +
> +    if ((match == NULL) || (priority < VIR_LOG_DEBUG) ||
> +        (priority > VIR_LOG_ERROR))
> +        return NULL;

Similar to previous patch, no reason for failure, but the following two
do provide a failure reason as would the failure from virCheckFlags. So
add some sort of error message if "priority" is out of range before
returning NULL

Also I would do the ATTRIBUTE_NONNULL(1) trick and avoid the failure due
to !match

> +
> +    if (VIR_STRDUP_QUIET(mdup, match) < 0)
> +        return NULL;
> +
> +    if (VIR_ALLOC_QUIET(ret) < 0) {
> +        VIR_FREE(mdup);
> +        return NULL;
> +    }
> +
> +    ret->match = mdup;
> +    ret->priority = priority;
> +    ret->flags = flags;
> +
> +    return ret;
> +}
> diff --git a/src/util/virlog.h b/src/util/virlog.h
> index fb32c41..a56d297 100644
> --- a/src/util/virlog.h
> +++ b/src/util/virlog.h
> @@ -232,5 +232,8 @@ virLogOutputPtr virLogOutputNew(virLogOutputFunc f,
>                                  virLogPriority priority,
>                                  virLogDestination dest,
>                                  const char *name);
> +virLogFilterPtr virLogFilterNew(const char *match,
> +                                virLogPriority priority,
> +                                unsigned int flags);

s/;/ ATTRIBUTE_NONNULL(1);

ACK with the adjustments,

John

>  
>  #endif
>