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 <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?

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