[Spice-devel] [PATCH spice-server v2 2/3] Dispatcher: typedef enum for dispatcher flags
Jonathon Jongsma
jjongsma at redhat.com
Wed Sep 6 16:12:00 UTC 2017
On Wed, 2017-09-06 at 07:47 -0400, Frediano Ziglio wrote:
> >
> > dispatcher_register_handler() accepts a flag value that determines
> > whether the message type requires an ACK or async_done handler.
> > This
> > parameter was previously just an int. It is now a typedef'ed enum:
> > DispatcherFlag. The enum values were accordingly renamed:
> >
> > DISPATCHER_* -> DISPATCHER_FLAG_*
> >
> > Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
>
> I honestly much prefer the "ack" field name instead of the "flag".
> "flag" is too generic.
>
> Maybe when I see ACK I think at TCP and I understand what it
> means. Why not instead using this enumeration?
>
> typedef enum {
> DISPATCHER_ACK_NONE = 0,
> DISPATCHER_ACK_SYNC,
> DISPATCHER_ACK_ASYNC
> } DispatcherAck;
>
> and
>
> DispatcherAck ack;
>
> (or ack_type)
Yeah, I thought about that. But I felt like the async_done thing was a
bit different than an 'ACK'. It allows you to call a function after the
message has been handled. But it doesn't really place any restrictions
on what that function does. In our case, it ends up calling the
QXLInterface::async_complete() function, which might update some
internal state and trigger an interrupt or something. But that seemed
like a bit of a broader concept than an 'ACK' to me. It's more like an
async callback.
I'm not sure that I'm totally happy with the 'flag' terminology either,
but at the time I thought it was better than ACK. But I don't feel too
strongly.
Jonathon
>
> Frediano
>
>
> > ---
> > server/dispatcher.c | 12 ++++-----
> > server/dispatcher.h | 20 +++++++--------
> > server/red-worker.c | 74
> > ++++++++++++++++++++++++++---------------------------
> > 3 files changed, 53 insertions(+), 53 deletions(-)
> >
> > diff --git a/server/dispatcher.c b/server/dispatcher.c
> > index 6f2c4d85e..76f5eeaf4 100644
> > --- a/server/dispatcher.c
> > +++ b/server/dispatcher.c
> > @@ -40,7 +40,7 @@ static void setup_dummy_signal_handler(void);
> >
> > typedef struct DispatcherMessage {
> > size_t size;
> > - int ack;
> > + DispatcherFlag flag;
> > dispatcher_handle_message handler;
> > } DispatcherMessage;
> >
> > @@ -303,13 +303,13 @@ static int
> > dispatcher_handle_single_read(Dispatcher
> > *dispatcher)
> > } else {
> > spice_printerr("error: no handler for message type %d",
> > type);
> > }
> > - if (msg->ack == DISPATCHER_ACK) {
> > + if (msg->flag == DISPATCHER_FLAG_ACK) {
> > if (write_safe(dispatcher->priv->recv_fd,
> > (uint8_t*)&ack, sizeof(ack)) == -1) {
> > spice_printerr("error writing ack for message %d",
> > type);
> > /* TODO: close socketpair? */
> > }
> > - } else if (msg->ack == DISPATCHER_ASYNC &&
> > dispatcher->priv->handle_async_done) {
> > + } else if (msg->flag == DISPATCHER_FLAG_ASYNC &&
> > dispatcher->priv->handle_async_done) {
> > dispatcher->priv->handle_async_done(dispatcher->priv-
> > >opaque, type,
> > payload);
> > }
> > return 1;
> > @@ -346,7 +346,7 @@ void dispatcher_send_message(Dispatcher
> > *dispatcher,
> > uint32_t message_type,
> > message_type);
> > goto unlock;
> > }
> > - if (msg->ack == DISPATCHER_ACK) {
> > + if (msg->flag == DISPATCHER_FLAG_ACK) {
> > if (read_safe(send_fd, (uint8_t*)&ack, sizeof(ack), 1) ==
> > -1) {
> > spice_printerr("error: failed to read ack");
> > } else if (ack != ACK) {
> > @@ -369,7 +369,7 @@ void dispatcher_register_async_done_callback(
> >
> > void dispatcher_register_handler(Dispatcher *dispatcher, uint32_t
> > message_type,
> > dispatcher_handle_message
> > handler,
> > - size_t size, int ack)
> > + size_t size, DispatcherFlag flag)
> > {
> > DispatcherMessage *msg;
> >
> > @@ -378,7 +378,7 @@ void dispatcher_register_handler(Dispatcher
> > *dispatcher,
> > uint32_t message_type,
> > msg = &dispatcher->priv->messages[message_type];
> > msg->handler = handler;
> > msg->size = size;
> > - msg->ack = ack;
> > + msg->flag = flag;
> > if (msg->size > dispatcher->priv->payload_size) {
> > dispatcher->priv->payload = realloc(dispatcher->priv-
> > >payload,
> > msg->size);
> > dispatcher->priv->payload_size = msg->size;
> > diff --git a/server/dispatcher.h b/server/dispatcher.h
> > index 97b01de9c..862fc46b9 100644
> > --- a/server/dispatcher.h
> > +++ b/server/dispatcher.h
> > @@ -72,11 +72,11 @@ typedef void
> > (*dispatcher_handle_async_done)(void
> > *opaque,
> > void dispatcher_send_message(Dispatcher *dispatcher, uint32_t
> > message_type,
> > void *payload);
> >
> > -enum {
> > - DISPATCHER_NONE = 0,
> > - DISPATCHER_ACK,
> > - DISPATCHER_ASYNC
> > -};
> > +typedef enum {
> > + DISPATCHER_FLAG_NONE = 0,
> > + DISPATCHER_FLAG_ACK,
> > + DISPATCHER_FLAG_ASYNC
> > +} DispatcherFlag;
> >
> > /*
> > * dispatcher_register_handler
> > @@ -84,16 +84,16 @@ enum {
> > * @messsage_type: message type
> > * @handler: message handler
> > * @size: message size. Each type has a fixed associated
> > size.
> > - * @ack: One of DISPATCHER_NONE, DISPATCHER_ACK,
> > DISPATCHER_ASYNC.
> > - * DISPATCHER_NONE - only send the message
> > - * DISPATCHER_ACK - send an ack after the message
> > - * DISPATCHER_ASYNC - call send an ack. This is
> > per message
> > type - you can't send the
> > + * @flag: One of DISPATCHER_FLAG_NONE,
> > DISPATCHER_FLAG_ACK,
> > DISPATCHER_FLAG_ASYNC.
> > + * DISPATCHER_FLAG_NONE - only send the message
> > + * DISPATCHER_FLAG_ACK - send an ack after the
> > message
> > + * DISPATCHER_FLAG_ASYNC - call send an ack. This
> > is per
> > message type - you can't send the
> > * same message type with and without. Register
> > two
> > different
> > * messages if that is what you want.
> > */
> > void dispatcher_register_handler(Dispatcher *dispatcher, uint32_t
> > message_type,
> > dispatcher_handle_message
> > handler, size_t
> > size,
> > - int ack);
> > + DispatcherFlag flag);
> >
> > /*
> > * dispatcher_register_async_done_callback
> > diff --git a/server/red-worker.c b/server/red-worker.c
> > index 4d8566be7..c3f2dbfa0 100644
> > --- a/server/red-worker.c
> > +++ b/server/red-worker.c
> > @@ -1024,187 +1024,187 @@ static void register_callbacks(Dispatcher
> > *dispatcher)
> > RED_WORKER_MESSAGE_DISPLAY_CONNECT
> > ,
> > handle_dev_display_connect,
> > sizeof(RedWorkerMessageDisplayConn
> > ect),
> > - DISPATCHER_NONE);
> > + DISPATCHER_FLAG_NONE);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_DISPLAY_DISCONN
> > ECT,
> > handle_dev_display_disconnect,
> > sizeof(RedWorkerMessageDisplayDisc
> > onnect),
> > - DISPATCHER_ACK);
> > + DISPATCHER_FLAG_ACK);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_DISPLAY_MIGRATE
> > ,
> > handle_dev_display_migrate,
> > sizeof(RedWorkerMessageDisplayMigr
> > ate),
> > - DISPATCHER_NONE);
> > + DISPATCHER_FLAG_NONE);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_CURSOR_CONNECT,
> > handle_dev_cursor_connect,
> > sizeof(RedWorkerMessageCursorConne
> > ct),
> > - DISPATCHER_NONE);
> > + DISPATCHER_FLAG_NONE);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_CURSOR_DISCONNE
> > CT,
> > handle_dev_cursor_disconnect,
> > sizeof(RedWorkerMessageCursorDisco
> > nnect),
> > - DISPATCHER_ACK);
> > + DISPATCHER_FLAG_ACK);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_CURSOR_MIGRATE,
> > handle_dev_cursor_migrate,
> > sizeof(RedWorkerMessageCursorMigra
> > te),
> > - DISPATCHER_NONE);
> > + DISPATCHER_FLAG_NONE);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_UPDATE,
> > handle_dev_update,
> > sizeof(RedWorkerMessageUpdate),
> > - DISPATCHER_ACK);
> > + DISPATCHER_FLAG_ACK);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_UPDATE_ASYNC,
> > handle_dev_update_async,
> > sizeof(RedWorkerMessageUpdateAsync
> > ),
> > - DISPATCHER_ASYNC);
> > + DISPATCHER_FLAG_ASYNC);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_ADD_MEMSLOT,
> > handle_dev_add_memslot,
> > sizeof(RedWorkerMessageAddMemslot)
> > ,
> > - DISPATCHER_ACK);
> > + DISPATCHER_FLAG_ACK);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_ADD_MEMSLOT_ASY
> > NC,
> > handle_dev_add_memslot_async,
> > sizeof(RedWorkerMessageAddMemslotA
> > sync),
> > - DISPATCHER_ASYNC);
> > + DISPATCHER_FLAG_ASYNC);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_DEL_MEMSLOT,
> > handle_dev_del_memslot,
> > sizeof(RedWorkerMessageDelMemslot)
> > ,
> > - DISPATCHER_NONE);
> > + DISPATCHER_FLAG_NONE);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_DESTROY_SURFACE
> > S,
> > handle_dev_destroy_surfaces,
> > sizeof(RedWorkerMessageDestroySurf
> > aces),
> > - DISPATCHER_ACK);
> > + DISPATCHER_FLAG_ACK);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_DESTROY_SURFACE
> > S_ASYNC,
> > handle_dev_destroy_surfaces_async,
> > sizeof(RedWorkerMessageDestroySurf
> > acesAsync),
> > - DISPATCHER_ASYNC);
> > + DISPATCHER_FLAG_ASYNC);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_DESTROY_PRIMARY
> > _SURFACE,
> > handle_dev_destroy_primary_surface
> > ,
> > sizeof(RedWorkerMessageDestroyPrim
> > arySurface),
> > - DISPATCHER_ACK);
> > + DISPATCHER_FLAG_ACK);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_DESTROY_PRIMARY
> > _SURFACE_ASYNC,
> > handle_dev_destroy_primary_surface
> > _async,
> > sizeof(RedWorkerMessageDestroyPrim
> > arySurfaceAsync),
> > - DISPATCHER_ASYNC);
> > + DISPATCHER_FLAG_ASYNC);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_CREATE_PRIMARY_
> > SURFACE_ASYNC,
> > handle_dev_create_primary_surface_
> > async,
> > sizeof(RedWorkerMessageCreatePrima
> > rySurfaceAsync),
> > - DISPATCHER_ASYNC);
> > + DISPATCHER_FLAG_ASYNC);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_CREATE_PRIMARY_
> > SURFACE,
> > handle_dev_create_primary_surface,
> > sizeof(RedWorkerMessageCreatePrima
> > rySurface),
> > - DISPATCHER_ACK);
> > + DISPATCHER_FLAG_ACK);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_RESET_IMAGE_CAC
> > HE,
> > handle_dev_reset_image_cache,
> > sizeof(RedWorkerMessageResetImageC
> > ache),
> > - DISPATCHER_ACK);
> > + DISPATCHER_FLAG_ACK);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_RESET_CURSOR,
> > handle_dev_reset_cursor,
> > sizeof(RedWorkerMessageResetCursor
> > ),
> > - DISPATCHER_ACK);
> > + DISPATCHER_FLAG_ACK);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_WAKEUP,
> > handle_dev_wakeup,
> > sizeof(RedWorkerMessageWakeup),
> > - DISPATCHER_NONE);
> > + DISPATCHER_FLAG_NONE);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_OOM,
> > handle_dev_oom,
> > sizeof(RedWorkerMessageOom),
> > - DISPATCHER_NONE);
> > + DISPATCHER_FLAG_NONE);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_START,
> > handle_dev_start,
> > sizeof(RedWorkerMessageStart),
> > - DISPATCHER_NONE);
> > + DISPATCHER_FLAG_NONE);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_FLUSH_SURFACES_
> > ASYNC,
> > handle_dev_flush_surfaces_async,
> > sizeof(RedWorkerMessageFlushSurfac
> > esAsync),
> > - DISPATCHER_ASYNC);
> > + DISPATCHER_FLAG_ASYNC);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_STOP,
> > handle_dev_stop,
> > sizeof(RedWorkerMessageStop),
> > - DISPATCHER_ACK);
> > + DISPATCHER_FLAG_ACK);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_LOADVM_COMMANDS
> > ,
> > handle_dev_loadvm_commands,
> > sizeof(RedWorkerMessageLoadvmComma
> > nds),
> > - DISPATCHER_ACK);
> > + DISPATCHER_FLAG_ACK);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_SET_COMPRESSION
> > ,
> > handle_dev_set_compression,
> > sizeof(RedWorkerMessageSetCompress
> > ion),
> > - DISPATCHER_NONE);
> > + DISPATCHER_FLAG_NONE);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_SET_STREAMING_V
> > IDEO,
> > handle_dev_set_streaming_video,
> > sizeof(RedWorkerMessageSetStreamin
> > gVideo),
> > - DISPATCHER_NONE);
> > + DISPATCHER_FLAG_NONE);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_SET_VIDEO_CODEC
> > S,
> > handle_dev_set_video_codecs,
> > sizeof(RedWorkerMessageSetVideoCod
> > ecs),
> > - DISPATCHER_NONE);
> > + DISPATCHER_FLAG_NONE);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_SET_MOUSE_MODE,
> > handle_dev_set_mouse_mode,
> > sizeof(RedWorkerMessageSetMouseMod
> > e),
> > - DISPATCHER_NONE);
> > + DISPATCHER_FLAG_NONE);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_DESTROY_SURFACE
> > _WAIT,
> > handle_dev_destroy_surface_wait,
> > sizeof(RedWorkerMessageDestroySurf
> > aceWait),
> > - DISPATCHER_ACK);
> > + DISPATCHER_FLAG_ACK);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_DESTROY_SURFACE
> > _WAIT_ASYNC,
> > handle_dev_destroy_surface_wait_as
> > ync,
> > sizeof(RedWorkerMessageDestroySurf
> > aceWaitAsync),
> > - DISPATCHER_ASYNC);
> > + DISPATCHER_FLAG_ASYNC);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_RESET_MEMSLOTS,
> > handle_dev_reset_memslots,
> > sizeof(RedWorkerMessageResetMemslo
> > ts),
> > - DISPATCHER_NONE);
> > + DISPATCHER_FLAG_NONE);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_MONITORS_CONFIG
> > _ASYNC,
> > handle_dev_monitors_config_async,
> > sizeof(RedWorkerMessageMonitorsCon
> > figAsync),
> > - DISPATCHER_ASYNC);
> > + DISPATCHER_FLAG_ASYNC);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_DRIVER_UNLOAD,
> > handle_dev_driver_unload,
> > sizeof(RedWorkerMessageDriverUnloa
> > d),
> > - DISPATCHER_NONE);
> > + DISPATCHER_FLAG_NONE);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_GL_SCANOUT,
> > handle_dev_gl_scanout,
> > sizeof(RedWorkerMessageGlScanout),
> > - DISPATCHER_NONE);
> > + DISPATCHER_FLAG_NONE);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_GL_DRAW_ASYNC,
> > handle_dev_gl_draw_async,
> > sizeof(RedWorkerMessageGlDraw),
> > - DISPATCHER_NONE);
> > + DISPATCHER_FLAG_NONE);
> > dispatcher_register_handler(dispatcher,
> > RED_WORKER_MESSAGE_CLOSE_WORKER,
> > handle_dev_close,
> > sizeof(RedWorkerMessageClose),
> > - DISPATCHER_NONE);
> > + DISPATCHER_FLAG_NONE);
> > }
> >
> >
> > --
> > 2.13.3
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> >
More information about the Spice-devel
mailing list