Re: [PATCH 1/1] multifd: Remove some redundant code

[Li Zhang]

On 12/17/21 10:39 AM, Claudio Fontana wrote:
> Hi Li,
>
> the full function for context:
>
> static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque)
> {
>      MultiFDSendParams *p = opaque;
>      QIOChannel *sioc = QIO_CHANNEL(qio_task_get_source(task));
>      Error *local_err = NULL;
>
>      trace_multifd_new_send_channel_async(p->id);
>      if (qio_task_propagate_error(task, &local_err)) {
>          goto cleanup;
>      } else {
>          p->c = QIO_CHANNEL(sioc);
>          qio_channel_set_delay(p->c, false);
>          p->running = true;
>          if (!multifd_channel_connect(p, sioc, local_err)) {
>              goto cleanup;
>          }
>          return;
>      }
>
> cleanup:
>      multifd_new_send_channel_cleanup(p, sioc, local_err);
> }
>
>
>
> On 12/17/21 10:33 AM, Li Zhang wrote:
> > Clean up some unnecessary code
> >
> > Signed-off-by: Li Zhang <lizhang@suse.de>
> > ---
> >   migration/multifd.c | 9 ++-------
> >   1 file changed, 2 insertions(+), 7 deletions(-)
> >
> > diff --git a/migration/multifd.c b/migration/multifd.c
> > index 3242f688e5..1405cf95b8 100644
> > --- a/migration/multifd.c
> > +++ b/migration/multifd.c
> > @@ -854,9 +854,7 @@ static void multifd_new_send_channel_async(QIOTask *task, 
> > gpointer opaque)
> >       Error *local_err = NULL;
> >   
> >       trace_multifd_new_send_channel_async(p->id);
> > -    if (qio_task_propagate_error(task, &local_err)) {
> > -        goto cleanup;
>
> I see you are removing this goto, but keeping there the other one.. is this a 
> bit inconsistent?

Ah, you are right. The other one is not necessary anymore.

> Should the second check be inverted too, to remove the other goto as well?
I would like to remove the goto and call the function directly.

> Ciao,
>
> Claudio
>
> > -    } else {
> > +    if (!qio_task_propagate_error(task, &local_err)) {
> >           p->c = QIO_CHANNEL(sioc);
> >           qio_channel_set_delay(p->c, false);
> >           p->running = true;
> > @@ -1078,10 +1076,7 @@ static void *multifd_recv_thread(void *opaque)
> >   
> >           ret = qio_channel_read_all_eof(p->c, (void *)p->packet,
> >                                          p->packet_len, &local_err);
> > -        if (ret == 0) {   /* EOF */
> > -            break;
> > -        }
> > -        if (ret == -1) {   /* Error */
> > +        if (ret == 0 || ret == -1) {   /* 0: EOF  -1: Error */
> >               break;
> >           }
> >