[libvirt] [PATCH v2 12/20] virlog: Introduce virLogParseFilters

[John Ferlan]

On 08/18/2016 07:47 AM, Erik Skultety wrote:
> Same as with outputs. Another layer of abstraction, this provides support for
> parsing a set of filters (preferred way).

Comment works well if you read the previous patch...

> 
> Signed-off-by: Erik Skultety <eskultet at redhat.com>
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virlog.c        | 65 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/util/virlog.h        |  1 +
>  3 files changed, 67 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index b12ca92..063fe5f 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1868,6 +1868,7 @@ virLogParseAndDefineFilters;
>  virLogParseAndDefineOutputs;
>  virLogParseDefaultPriority;
>  virLogParseFilter;
> +virLogParseFilters;
>  virLogParseOutput;
>  virLogParseOutputs;
>  virLogPriorityFromSyslog;
> diff --git a/src/util/virlog.c b/src/util/virlog.c
> index 89e58cd..4b2aa4d 100644
> --- a/src/util/virlog.c
> +++ b/src/util/virlog.c
> @@ -2045,3 +2045,68 @@ virLogParseOutputs(const char *src, virLogOutputPtr **outputs)
>      virStringFreeList(strings);
>      return ret;
>  }
> +
> +/**
> + * virLogParseFilters:
> + * @src: string defining a (set of) filter(s)
> + * @filters: pointer to a list where the individual filters shall be parsed
> + *
> + * The format for a filter is:
> + *    x:name
> + *       where name is a match string
> + * the x prefix is the minimal level where the messages should be logged
> + *    1: DEBUG
> + *    2: INFO
> + *    3: WARNING
> + *    4: ERROR
> + *
> + * This method parses @src and produces a list of individual filters which then
> + * needs to be passed to virLogDefineFilters in order to be set and taken into
> + * effect.
> + * Multiple filters can be defined in a single @src, they just need to be
> + * separated by spaces.
> + *

most of the above could be moved to patch 10.

> + * Returns the number of filter parsed or -1 in case of error.
> + */
> +int
> +virLogParseFilters(const char *src, virLogFilterPtr **filters)
> +{
> +    int ret = -1;
> +    size_t count = 0;
> +    size_t i;
> +    char **strings = NULL;
> +    virLogFilterPtr filter = NULL;
> +    virLogFilterPtr *list = NULL;
> +
> +    if (!src)
> +        return -1;

Use ATTRIBUTE_NONNULL(1) in prototype.

> +
> +    VIR_DEBUG("filters=%s", src);
> +
> +    if (!(strings = virStringSplit(src, " ", 0)))
> +        goto cleanup;
> +
> +    for (i = 0; strings[i]; i++) {

Similar comment about using virStringSplitCount function

> +        /* virStringSplit may return empty strings */
> +        if (STREQ(strings[i], ""))
> +            continue;
> +
> +        if (!(filter = virLogParseFilter(strings[i])))
> +            goto cleanup;
> +
> +        if (VIR_APPEND_ELEMENT(list, count, filter)) {
> +            virLogFilterFree(filter);
> +            goto cleanup;
> +        }
> +
> +        virLogFilterFree(filter);

Not sure this is necessary unless you use the _COPY append macro

> +    }
> +
> +    ret = count;
> +    *filters = list;

If you set list = NULL, then...

> + cleanup:
> +    if (ret < 0)

... the ret < 0 check is not necessary

> +        virLogFilterListFree(list, count);
> +    virStringFreeList(strings);
> +    return ret;
> +}
> diff --git a/src/util/virlog.h b/src/util/virlog.h
> index ed60c00..9ccc650 100644
> --- a/src/util/virlog.h
> +++ b/src/util/virlog.h
> @@ -248,5 +248,6 @@ virLogOutputPtr virLogNewOutputToJournald(int priority);
>  virLogOutputPtr virLogParseOutput(const char *src);
>  virLogFilterPtr virLogParseFilter(const char *src);
>  int virLogParseOutputs(const char *src, virLogOutputPtr **outputs);
> +int virLogParseFilters(const char *src, virLogFilterPtr **filters);

s/;/ATTRIBUTE_NONNULL(1);

ACK w/ adjustments

John
>  
>  #endif
>