[Spice-devel] [PATCH 4/9] worker: remove useless MESSAGE_READY

[Frediano Ziglio]

> 
> From: Marc-André Lureau <marcandre.lureau at gmail.com>
> 
> Now that worker is created before running, and run() returns success,
> there is no point in using MESSAGE_READY.
> ---
>  server/red_dispatcher.h | 10 ++++------
>  server/red_worker.c     |  8 +-------
>  2 files changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/server/red_dispatcher.h b/server/red_dispatcher.h
> index 2121007..ae46982 100644
> --- a/server/red_dispatcher.h
> +++ b/server/red_dispatcher.h
> @@ -63,11 +63,6 @@ static inline void send_data(int fd, void *in_buf, int n)
>      } while (n);
>  }
>  
> -static inline void write_message(int fd, RedWorkerMessage *message)
> -{
> -    send_data(fd, message, sizeof(RedWorkerMessage));
> -}
> -
>  static inline void receive_data(int fd, void *in_buf, int n)
>  {
>      uint8_t *buf = in_buf;
> @@ -84,12 +79,15 @@ static inline void receive_data(int fd, void *in_buf, int
> n)
>      } while (n);
>  }
>  
> +/* Keep message order, only append new messages! */
>  enum {
>      RED_WORKER_MESSAGE_NOP,
> +
>      RED_WORKER_MESSAGE_UPDATE,
>      RED_WORKER_MESSAGE_WAKEUP,
>      RED_WORKER_MESSAGE_OOM,
> -    RED_WORKER_MESSAGE_READY,
> +    RED_WORKER_MESSAGE_READY, /* unused */
> +

At the beginning I didn't understand why is not possible to remove it.
This is not an ABI and used only internally.
Actually the only reason (I think) is that replay feature store integer values
for this enumeration so it became an ABI.
Personally... I don't like it!
But this will require a patch for the replay stuff (like using names and fallback
to number only for old captures).
For the time being I would add the reason to the comment.
Is it the only reason ?

>      RED_WORKER_MESSAGE_DISPLAY_CONNECT,
>      RED_WORKER_MESSAGE_DISPLAY_DISCONNECT,
>      RED_WORKER_MESSAGE_DISPLAY_MIGRATE,
> diff --git a/server/red_worker.c b/server/red_worker.c
> index daa233b..e5486c4 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -11853,7 +11853,6 @@ static void handle_dev_input(int fd, int event, void
> *opaque)
>  RedWorker* red_worker_new(WorkerInitData *init_data)
>  {
>      RedWorker *worker = spice_new0(RedWorker, 1);
> -    RedWorkerMessage message;
>      Dispatcher *dispatcher;
>      int i;
>      const char *record_filename;
> @@ -11933,8 +11932,6 @@ RedWorker* red_worker_new(WorkerInitData *init_data)
>      if (!spice_timer_queue_create()) {
>          spice_error("failed to create timer queue");
>      }
> -    message = RED_WORKER_MESSAGE_READY;
> -    write_message(worker->channel, &message);
>  
>      red_init_quic(worker);
>      red_init_lz(worker);
> @@ -12033,7 +12030,6 @@ SPICE_GNUC_NORETURN static void *red_worker_main(void
> *arg)
>  
>  bool red_worker_run(RedWorker *worker)
>  {
> -    uint32_t message;
>      sigset_t thread_sig_mask;
>      sigset_t curr_sig_mask;
>      int r;
> @@ -12051,7 +12047,5 @@ bool red_worker_run(RedWorker *worker)
>      }
>      pthread_sigmask(SIG_SETMASK, &curr_sig_mask, NULL);
>  
> -    message =
> dispatcher_read_message(red_dispatcher_get_dispatcher(worker->red_dispatcher));
> -
> -    return message == RED_WORKER_MESSAGE_READY;
> +    return r == 0;
>  }

Here note that there was a missing check (r was not tested).

Frediano