diff mbox series

[1/1] multifd: Remove some redundant code

Message ID 20211217093318.6260-1-lizhang@suse.de (mailing list archive)
State New, archived
Headers show
Series [1/1] multifd: Remove some redundant code | expand

Commit Message

Li Zhang Dec. 17, 2021, 9:33 a.m. UTC 
Clean up some unnecessary code

Signed-off-by: Li Zhang <lizhang@suse.de>
---
 migration/multifd.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

Comments

Claudio Fontana Dec. 17, 2021, 9:39 a.m. UTC | #1 
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?

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;
>          }
>  
>
Li Zhang Dec. 17, 2021, 9:53 a.m. UTC | #2 
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;
>>           }
>>   
>>
>
diff mbox series

Patch

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