Re: [libvirt] [PATCH 6/8] admin: Introduce virAdmConnectSetLoggingOutputs
> };
> diff --git a/src/util/virlog.c b/src/util/virlog.c
> index 4ac72dc..d04a461 100644
> --- a/src/util/virlog.c
> +++ b/src/util/virlog.c
> @@ -1820,6 +1820,8 @@ virLogParseFilters(const char *src, virLogFilterPtr
**filters)
> * @outputs: string defining a (set of) output(s)
> *
> * Replaces the current set of defined outputs with a new set of outputs.
> + * Should the set be empty, a default output is used according to the daemon's
> + * runtime attributes.
> *
> * Returns 0 on success or -1 in case of an error.
> */
> @@ -1828,12 +1830,13 @@ virLogSetOutputs(const char *src)
> {
> int ret = -1;
> int noutputs = 0;
> + const char *outputstr = *src ? src : virLogGetDefaultOutput();
Not sure this is right - virAdmConnectSetLoggingOutputs says 'outputs'
must be NonNull.
I assume the generated remoteAdminConnectSetLoggingOutputs will end up
calling adminConnectSetLoggingOutputs with a NonNull 'outputs'.
Thus this code gets called with NonNull outputs.
So, what exactly are you proposing? Because what I understand from it now is
that I should probably allow passing NULL outputs in
virAdmConnectSetLoggingOutputs as a way of resetting the outputs to our defaults
(as an addition to "").
Anyway, what you're really looking to do is check if the contents
of
'outputs' is empty, then use some default value. In this case, since
this code "owns" virLogDefaultOutput is there really a reason to call
the API?
True, I certainly don't have to make the call.
NB: even though virlog.h says outputs cannot be passed as NULL that's
somewhat of a misnomer since all the compiler is checking is that the
argument in the call stack isn't NULL - it doesn't check if the argument
being passed in actually NULL.
Yes, but this should be actual check for NULL should be the case when we're
handling user-passed values in which case as it is right now, there's no need,
user can't pass NULL and in all the other cases, our internal callers should
respect the ATTRIBUTE_NONNULL constraint. Of course, if you were suggesting to
actually allow users to pass NULL to the public API^^, then yes, this has to be
adjusted properly.
Thanks,
Erik
John
> virLogOutputPtr *outputs = NULL;
>
> if (virLogInitialize() < 0)
> return -1;
>
> - if ((noutputs = virLogParseOutputs(src, &outputs)) < 0)
> + if ((noutputs = virLogParseOutputs(outputstr, &outputs)) < 0)
> goto cleanup;
>
> if (virLogDefineOutputs(outputs, noutputs) < 0)
>
Attachments:
- signature.asc (application/pgp-signature — 819 bytes)