> > };
> > 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) > >
signature.asc
Description: PGP signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
