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

[Li Zhang <lizhang@suse.de>]

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