> > };
> > 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)
> >
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
