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

[Claudio Fontana]

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 <lizh...@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?

Should the second check be inverted too, to remove the other goto as well?

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;
>          }
>  
>