Re: [PATCH v7 4/5] migration: Add migrate_use_tls() helper

[Peter Xu]

On Wed, Jan 19, 2022 at 03:06:55PM -0300, Leonardo Bras Soares Passos wrote:
> Hello Peter,
> 
> On Thu, Jan 13, 2022 at 4:02 AM Peter Xu <peterx@redhat.com> wrote:
> >
> > On Thu, Jan 06, 2022 at 07:13:41PM -0300, Leonardo Bras wrote:
> > >  void migration_channel_process_incoming(QIOChannel *ioc)
> > >  {
> > > -    MigrationState *s = migrate_get_current();
> > >      Error *local_err = NULL;
> > >
> > >      trace_migration_set_incoming_channel(
> > >          ioc, object_get_typename(OBJECT(ioc)));
> > >
> > > -    if (s->parameters.tls_creds &&
> > > -        *s->parameters.tls_creds &&
> > > +    if (migrate_use_tls() &&
> > >          !object_dynamic_cast(OBJECT(ioc),
> > >                               TYPE_QIO_CHANNEL_TLS)) {
> > > +        MigrationState *s = migrate_get_current();
> > > +
> >
> > Trivial nit: I'd rather keep the line there; as the movement offers nothing,
> > imho..
> 
> The idea to move the 's' to inside the if  block is to make it clear
> it's only used in this case.

IMHO not necessary; I hardly read declarations for this, unless there's a bug,
e.g. on variable shadowing. Moving it downwards makes it easier to happen. :)

> 
> But if you think it's better to keep it at the beginning of the
> function, sure, I can change that.
> Just let me know.

Since there'll be a new version, that definitely looks nicer.

Thanks,

-- 
Peter Xu