Re: [PATCH v2 03/24] migration: Normalize tls arguments

[Peter Xu <peterx@redhat.com>]

On Fri, Jul 04, 2025 at 10:12:33AM -0300, Fabiano Rosas wrote:

[...]

> >>> +static void tls_opt_to_str(StrOrNull **tls_opt)
> >>> +{
> >>> +    StrOrNull *opt = *tls_opt;
> >>> +
> >>> +    if (!opt) {
> >>> +        return;
> >>
> >> ... it can also be null.
> >>
> >
> > Hmm, I'll have to double check, but with the StrOrNull property being
> > initialized, NULL should not be possible. This looks like a mistake.
> >
> 
> The code is correct, this is coming from the QAPI, so it could be NULL
> in case the user hasn't provided the option. I'll use your suggested
> wording and the code suggestion as well.

One more trivial question:

> 
> >> Maybe
> >>
> >>    /* Normalize QTYPE_QNULL to QTYPE_QSTRING "" */
> >>
> >>> +    }
> >>> +
> >>> +    switch (opt->type) {
> >>> +    case QTYPE_QSTRING:
> >>> +        return;
> >>> +    case QTYPE_QNULL:
> >>> +        qobject_unref(opt->u.n);
> >>> +        break;
> >>> +    default:
> >>> +        g_assert_not_reached();
> >>> +    }
> >>> +
> >>> +    opt->type = QTYPE_QSTRING;
> >>> +    opt->u.s = g_strdup("");
> >>> +    *tls_opt = opt;

Does tls_opt ever change?   I wonder if this line is not needed, instead
tls_opt_to_str() can take an "StrOrNull *opt" directly.

> >>> +}
> >>
> >> I'd prefer something like
> >>
> >>        if (!opt || opt->type == QTYPE_QSTRING) {
> >>            return;
> >>        }
> >>        qobject_unref(opt->u.n);
> >>        opt->type = QTYPE_QSTRING;
> >>        opt->u.s = g_strdup("");
> >>        *tls_opt = opt;
> >>
> >> But this is clearly a matter of taste.
> 
> This is better indeed. I was moving back-and-forth between
> implementations and the code ended up a bit weird. Thanks!
> 

-- 
Peter Xu