Hi Frediano,
On Tue, 2016-10-11 at 09:55 -0400, Frediano Ziglio wrote:
> >
> > On Mon, 2016-10-10 at 12:48 -0400, Frediano Ziglio wrote:
> > > >
> > > > Subject: [PATCH spice-server v2 7/8] Convert
> > > > RedChannel heirarchy to GObject
> > > >
> > >
> > > Small spell, it's "hierarchy".
> > >
> > > >
> > > > From: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> > > >
> > > > FIXME: this commit appears to introduce a vdagent-related
> > > > crash in
> > > > vdi_port_read_one_msg_from_device(). sin->st is NULL.
> > >
> > > If this is still true I would fix it.
> >
> > I added this comment a long time ago when I was running into a
> > crash
> > while testing the refactory branch. I bisected and ended up at
> > this
> > commit, but didn't have time to fix the crash right away, so I
> > added
> > the comment to remind myself. However, I have not seen the crash
> > at all
> > recently.
> >
> > My theory is that perhaps this crash was caused by a bad rebase
> > that
> > introduced an error that we've subsequently fixed due to reviews
> > and
> > additional rebasing.
> >
> >
> > >
> > > > @@ -381,7 +367,7 @@ void
> > > > cursor_channel_process_cmd(CursorChannel
> > > > *cursor,
> > > > RedCursorCmd *cursor_cmd)
> > > >
> > > > void cursor_channel_reset(CursorChannel *cursor)
> > > > {
> > > > - RedChannel *channel = &cursor->common.base;
> > > > + RedChannel *channel = RED_CHANNEL(cursor);
> > > >
> > > > spice_return_if_fail(cursor);
> > > >
> > > > @@ -395,9 +381,9 @@ void cursor_channel_reset(CursorChannel
> > > > *cursor)
> > > > if
> > > > (!common_graphics_channel_get_during_target_migrate(C
> > > > OMMON
> > > > _GRAPHICS_CHANNEL(cursor)))
> > > > {
> > > > red_pipes_add_verb(channel,
> > > > SPICE_MSG_CURSOR_RESET);
> > > > }
> > > > - if (!red_channel_wait_all_sent(&cursor->common.base,
> > > > + if (!red_channel_wait_all_sent(RED_CHANNEL(cursor),
> > > > COMMON_CLIENT_TIMEOUT)
> > > > ) {
> > > > - red_channel_apply_clients(channel,
> > > > + red_channel_apply_clients(RED_CHANNEL(cursor),
> > >
> > > Why not using channel variable ??
> > >
> >
> >
> > Strange. Not sure. Will change.
> >
> > > >
> > > > red_channel_client_disc
> > > > onnec
> > > > t_if_pending_send);
> > > > }
> > > > }
> > > > @@ -407,7 +393,7 @@ static void
> > > > cursor_channel_init_client(CursorChannel
> > > > *cursor, CursorChannelClien
> > > > {
> > > > spice_return_if_fail(cursor);
> > > >
> > > > - if (!red_channel_is_connected(&cursor->common.base)
> > > > + if (!red_channel_is_connected(RED_CHANNEL(cursor))
> > > > ||
> > > > common_graphics_channel_get_during_target_migrate(COMMON_GRAPH
> > > > ICS_C
> > > > HANNEL(cursor)))
> > > > || {
> > > > spice_debug("during_target_migrate: skip init");
> > > > return;
> > > > @@ -420,7 +406,7 @@ static void
> > > > cursor_channel_init_client(CursorChannel
> > > > *cursor, CursorChannelClien
> > > > red_channel_pipes_add_type(RED_CHANNEL(cursor),
> > > > RED_PIPE_ITEM_TYPE_CURSOR_INIT);
> > > > }
> > > >
> > > > -void cursor_channel_init(CursorChannel *cursor)
> > > > +void cursor_channel_do_init(CursorChannel *cursor)
> > > > {
> > > > cursor_channel_init_client(cursor, NULL);
> > > > }
> > > > @@ -454,3 +440,25 @@ void cursor_channel_connect(CursorChannel
> > > > *cursor,
> > > > RedClient *client, RedsStream
> > > >
> > > > cursor_channel_init_client(cursor, ccc);
> > > > }
> > > > +
> > > > +static void
> > > > +cursor_channel_class_init(CursorChannelClass *klass)
> > > > +{
> > > > + RedChannelClass *channel_class =
> > > > RED_CHANNEL_CLASS(klass);
> > > > +
> > > > + g_type_class_add_private(klass,
> > > > sizeof(CursorChannelPrivate));
> > > > +
> > > > + channel_class->parser =
> > > > spice_get_client_channel_parser(SPICE_CHANNEL_CURSOR, NULL);
> > > > + channel_class->handle_parsed =
> > > > red_channel_client_handle_message;
> > > > +
> > > > + channel_class->on_disconnect
> > > > = cursor_channel_client_on_disconnect;
> > > > + channel_class->send_item = cursor_channel_send_item;
> > > > +}
> > > > +
> > > > +static void
> > > > +cursor_channel_init(CursorChannel *self)
> > > > +{
> > > > + self->priv = CURSOR_CHANNEL_PRIVATE(self);
> > > > + self->priv->cursor_visible = TRUE;
> > > > + self->priv->mouse_mode = SPICE_MOUSE_MODE_SERVER;
> > > > +}
> > > > diff --git a/server/cursor-channel.h b/server/cursor-channel.h
> > > > index a3ddaa3..8b3bc17 100644
> > > > --- a/server/cursor-channel.h
> > > > +++ b/server/cursor-channel.h
> > > > @@ -19,6 +19,17 @@
> > > > # define CURSOR_CHANNEL_H_
> > > >
> > > > #include "common-graphics-channel.h"
> > > > +#include "red-parse-qxl.h"
> > > > +
> > > > +G_BEGIN_DECLS
> > > > +
> > > > +#define TYPE_CURSOR_CHANNEL cursor_channel_get_type()
> > > > +
> > > > +#define CURSOR_CHANNEL(obj)
> > > > (G_TYPE_CHECK_INSTANCE_CAST((obj),
> > > > TYPE_CURSOR_CHANNEL, CursorChannel))
> > > > +#define CURSOR_CHANNEL_CLASS(klass)
> > > > (G_TYPE_CHECK_CLASS_CAST((klass),
> > > > TYPE_CURSOR_CHANNEL, CursorChannelClass))
> > > > +#define IS_CURSOR_CHANNEL(obj)
> > > > (G_TYPE_CHECK_INSTANCE_TYPE((obj),
> > > > TYPE_CURSOR_CHANNEL))
> > > > +#define IS_CURSOR_CHANNEL_CLASS(klass)
> > > > (G_TYPE_CHECK_CLASS_TYPE((klass),
> > > > TYPE_CURSOR_CHANNEL))
> > > > +#define CURSOR_CHANNEL_GET_CLASS(obj)
> > > > (G_TYPE_INSTANCE_GET_CLASS((obj),
> > > > TYPE_CURSOR_CHANNEL, CursorChannelClass))
> > > >
> > > > /**
> > > > * This type it's a RedChannel class which implement cursor
> > > > (mouse)
> > > > @@ -26,6 +37,22 @@
> > > > * A pointer to CursorChannel can be converted to a
> > > > RedChannel.
> > > > */
> > > > typedef struct CursorChannel CursorChannel;
> > > > +typedef struct CursorChannelClass CursorChannelClass;
> > > > +typedef struct CursorChannelPrivate CursorChannelPrivate;
> > > > +
> > > > +struct CursorChannel
> > > > +{
> > > > + CommonGraphicsChannel parent;
> > > > +
> > > > + CursorChannelPrivate *priv;
> > > > +};
> > > > +
> > > > +struct CursorChannelClass
> > > > +{
> > > > + CommonGraphicsChannelClass parent_class;
> > > > +};
> > > > +
> > > > +GType cursor_channel_get_type(void) G_GNUC_CONST;
> > > >
> > >
> > > Why we need to expose this here ? On the C file is not enough ?
> >
> > This is used above in all of the 'standard' GObject macros (e.g.
> > CURSOR_CHANNEL(), IS_CURSOR_CHANNEL(), etc). It's standard
> > practice to
> > expose it in the header. In this case, it's possible that we could
> > move
> > it down to the .c file, depending on whether any other files use
> > these
> > macros, but I don't see much advantage to doing so. I'd prefer to
> > leave
> > the standard GObject boilerplate here.
> >
>
> Some months ago my sister decided it was time to change the car.
> As she has some child she decided for a bigger car. She was also
> thinking
> about having additional space to allow to go to holidays. After
> viewing
> different car we discussed if it was worth buying a bigger car
> considering
> it was more expensive to buy and to maintain (insurance and fuel).
> At the end she decided to buy a car enough for the child and family
> but that was not worth buying some more big one just for the holiday
> opting for a car roof bag.
> The 'standard' solution for the car is buying a bigger car but not
> all standard solutions are the best.
> The main reason why the standard is that way is that it supports
> more
> cases and so it's easier to put in a guide or tutorial or a book...
> just that we are not writing a book or following a tutorial or
> guide.
>
> Exposing CursorChannel and CursorChannelClass in the header is a way
> to tell that this is the object structure and can be used for
> instance
> for inheritance. Some languages introduced the "final" keyword to
> avoid
> inheritance... C just do not support objects so not exposing the
> structure it's a way to use.
> Also is there is no need to expose the structure why to do it?
> Another reason in favor of not exposing it is that you could avoid
> to
> pay the penalty (memory, code and cpu) of the "priv" field.
>
> About all that macros boilerplate I don't understand why they
> don't came to a better solution, something like
>
> G_TYPE_CAST(pointer, RedChannel)
>
> instead of having to define all these macro for every object...
Maybe we can use G_DECLARE_FINAL_TYPE
https://developer.gnome.org/gobject/unstable/gobject-Type-Information.
html#G-DECLARE-FINAL-TYPE:CAPS
In general I prefer to expose declarations only if needed
Pavel
> Require that if there is a GObject typename "RedChannel" you
> have a RedChannel_get_type() function does not seem so hard
> or cause so many problems (but probably I miss something).
> Also instead of
>
> if (IS_CURSOR_CHANNEL(obj))
>
> why not using
>
> if (CURSOR_CHANNEL(obj))
>
> and avoid to define 2 macros?
>
>
> As macro only users of these headers can use them and we
> are the only users.
> So can't we define our macros so we don't have all that
> boilerplate?
> Are we expecting to export these GObject outside our code or
> use then in an higher level language such as Java or a visual
> tool?
>
> >
> > > > @@ -185,8 +185,9 @@ static void
> > > > red_display_add_image_to_pixmap_cache(RedChannelClient *rcc,
> > > > SpiceImage
> > > > *image,
> > > > SpiceImage
> > > > *io_image,
> > > > int
> > > > is_lossy)
> > > > {
> > > > + DisplayChannel *display_channel =
> > > > + DISPLAY_CHANNEL(red_channel_client_get_channel(rcc));
> > > > DisplayChannelClient *dcc = DISPLAY_CHANNEL_CLIENT(rcc);
> > > > - DisplayChannel *display_channel = DCC_TO_DC(dcc);
> > > >
> > >
> > > I prefer the old version, DCC_TO_DC is still present and used.
> >
> > Hmm, I guess I prefer the new version, even though it's a little
> > more
> > verbose. I generally prefer using standard naming conventions
> > instead
> > of excessive abbreviations that can be unclear (DCC_TO_DC). In
> > fact I'd
> > prefer to eventually get rid of the use of 'dcc' altogether and
> > switch
> > to DisplayChannelClient.
> >
> > >
> > > > diff --git a/server/display-channel.c b/server/display-
> > > > channel.c
> > > > index 69edd35..19f0aca 100644
> > > > --- a/server/display-channel.c
> > > > +++ b/server/display-channel.c
> > > > @@ -20,7 +20,131 @@
> > > >
> > > > #include <common/sw_canvas.h>
> > > >
> > > > -#include "display-channel.h"
> > > > +#include "display-channel-private.h"
> > > > +
> > > > +G_DEFINE_TYPE(DisplayChannel, display_channel,
> > > > TYPE_COMMON_GRAPHICS_CHANNEL)
> > > > +
> > > > +enum {
> > > > + PROP0,
> > > > + PROP_N_SURFACES,
> > > > + PROP_VIDEO_CODECS
> > > > +};
> > > > +
> > > > +static void
> > > > +display_channel_get_property(GObject *object,
> > > > + guint property_id,
> > > > + GValue *value,
> > > > + GParamSpec *pspec)
> > > > +{
> > > > + DisplayChannel *self = DISPLAY_CHANNEL(object);
> > > > +
> > > > + switch (property_id)
> > > > + {
> > > > + case PROP_N_SURFACES:
> > > > + g_value_set_uint(value, self->priv->n_surfaces);
> > > > + break;
> > > > + default:
> > > > + G_OBJECT_WARN_INVALID_PROPERTY_ID(object,
> > > > property_id,
> > > > pspec);
> > > > + }
> > > > +}
> > > > +
> > > > +static void
> > > > +display_channel_set_property(GObject *object,
> > > > + guint property_id,
> > > > + const GValue *value,
> > > > + GParamSpec *pspec)
> > > > +{
> > > > + DisplayChannel *self = DISPLAY_CHANNEL(object);
> > > > +
> > > > + switch (property_id)
> > > > + {
> > > > + case PROP_N_SURFACES:
> > > > + self->priv->n_surfaces = g_value_get_uint(value);
> > > > + break;
> > >
> > > Sure we want this writable ?
> > > Shouldn't we reallocate the array ?
> >
> > Well, if you look at where the property is defined, it's defined
> > as a
> > construct-only parameter. So although it is technically writable,
> > it's
> > only writable at construction. So it can't really be written after
> > that.
> >
> > >
> > > > @@ -2113,6 +2204,50 @@ void
> > > > display_channel_reset_image_cache(DisplayChannel
> > > > *self)
> > > > image_cache_reset(&self->priv->image_cache);
> > > > }
> > > >
> > > > +static void
> > > > +display_channel_class_init(DisplayChannelClass *klass)
> > > > +{
> > > > + GObjectClass *object_class = G_OBJECT_CLASS(klass);
> > > > + RedChannelClass *channel_class =
> > > > RED_CHANNEL_CLASS(klass);
> > > > +
> > > > + g_type_class_add_private(klass,
> > > > sizeof(DisplayChannelPrivate));
> > > > +
> > > > + object_class->get_property =
> > > > display_channel_get_property;
> > > > + object_class->set_property =
> > > > display_channel_set_property;
> > > > + object_class->constructed = display_channel_constructed;
> > > > + object_class->finalize = display_channel_finalize;
> > > > +
> > > > + channel_class->parser =
> > > > spice_get_client_channel_parser(SPICE_CHANNEL_DISPLAY, NULL);
> > > > + channel_class->handle_parsed = dcc_handle_message;
> > > > +
> > > > + channel_class->on_disconnect = on_disconnect;
> > > > + channel_class->send_item = dcc_send_item;
> > > > + channel_class->handle_migrate_flush_mark =
> > > > handle_migrate_flush_mark;
> > > > + channel_class->handle_migrate_data = handle_migrate_data;
> > > > + channel_class->handle_migrate_data_get_serial =
> > > > handle_migrate_data_get_serial;
> > > > + channel_class->config_socket = dcc_config_socket;
> > > > +
> > > > + g_object_class_install_property(object_class,
> > > > + PROP_N_SURFACES,
> > > > + g_param_spec_uint("n-
> > > > surfaces",
> > > > + "number
> > > > of
> > > > surfaces",
> > > > + "Number
> > > > of
> > > > surfaces
> > > > for this channel",
> > > > + 0,
> > > > G_MAXUINT,
> > > > + 0,
> > > > + G_PARAM
> > > > _CONS
> > > > TRUCT_ONLY
> > > > >
> > > > >
> > > >
> > > > + G_PARAM
> > > > _READ
> > > > WRITE |
> > > > +
> > >
> > > Imo should be write only.
> > >
> >
> > As mentioned above, G_PARAM_CONSTRUCT_ONLY restricts it to be
> > writable
> > only at construct time. But it needs to be READWRITE if we want to
> > pass
> > it as a construction parameter.
> >
>
> Yes, you are right.
>
> > > >
> > > > diff --git a/server/display-channel.h b/server/display-
> > > > channel.h
> > > > index 3762e54..9ac9046 100644
> > > > --- a/server/display-channel.h
> > > > +++ b/server/display-channel.h
> > > > @@ -20,32 +20,59 @@
> > > >
> > > > #include <setjmp.h>
> > > > #include <common/rect.h>
> > > > +#include <common/sw_canvas.h>
> > > >
> > > > -#include "reds-stream.h"
> > > > #include "cache-item.h"
> > > > -#include "pixmap-cache.h"
> > > > -#include "stat.h"
> > > > -#include "reds.h"
> > > > -#include "memslot.h"
> > > > -#include "red-parse-qxl.h"
> > > > -#include "red-record-qxl.h"
> > > > +#include "image-encoders.h"
> > > > +#include "dcc.h"
> > > > #include "demarshallers.h"
> > > > -#include "red-channel.h"
> > > > -#include "red-qxl.h"
> > > > #include "dispatcher.h"
> > > > #include "main-channel.h"
> > > > -#include "migration-protocol.h"
> > > > #include "main-dispatcher.h"
> > > > +#include "memslot.h"
> > > > +#include "migration-protocol.h"
> > > > +#include "video-encoder.h"
> > > > +#include "pixmap-cache.h"
> > > > +#include "red-channel.h"
> > > > +#include "red-qxl.h"
> > > > +#include "red-parse-qxl.h"
> > > > +#include "red-record-qxl.h"
> > > > +#include "reds-stream.h"
> > > > +#include "reds.h"
> > > > #include "spice-bitmap-utils.h"
> > > > -#include "image-cache.h"
> > > > -#include "utils.h"
> > > > -#include "tree.h"
> > > > +#include "stat.h"
> > > > #include "stream.h"
> > > > -#include "dcc.h"
> > > > -#include "image-encoders.h"
> > > > #include "common-graphics-channel.h"
> > > > +#include "tree.h"
> > > > +#include "utils.h"
> > > > +
> > >
> > > May headers are just included in a different order.
> > > Is there any reason to change the order?
> > >
> >
> > I suspect that I may have alphabetized them here since there were
> > already significant changes. But we could revert that.
> >
> > > >
> > > > diff --git a/server/dummy-channel.c b/server/dummy-channel.c
> > > > new file mode 100644
> > > > index 0000000..6ec7842
> > > > --- /dev/null
> > > > +++ b/server/dummy-channel.c
> > > > @@ -0,0 +1,58 @@
> > > > +/* dummy-channel.c */
> > > > +#ifdef HAVE_CONFIG_H
> > > > +#include <config.h>
> > > > +#endif
> > > > +
> > > > +#include "dummy-channel.h"
> > > > +
> > > > +G_DEFINE_TYPE(DummyChannel, dummy_channel, RED_TYPE_CHANNEL)
> > > > +
> > > > +#define DUMMY_CHANNEL_PRIVATE(o)
> > > > (G_TYPE_INSTANCE_GET_PRIVATE((o),
> > > > TYPE_DUMMY_CHANNEL, DummyChannelPrivate))
> > > > +
> > > > +struct DummyChannelPrivate
> > > > +{
> > > > + gpointer padding;
> > > > +};
> > > > +
> > >
> > > Why just not using a private ? This padding field is not used.
> > > And you can just keep the priv field to retain ABI.
> > > This dummy stuff is moving from ugly to disgusting...
> >
> > yeah, I'm not sure why I did that, to be honest.
> >
> > >
> > > > diff --git a/server/inputs-channel.c b/server/inputs-channel.c
> > > > index 83c1360..c351dad 100644
> > > > --- a/server/inputs-channel.c
> > > > +++ b/server/inputs-channel.c
> > > > @@ -57,6 +57,83 @@
> > > > #define RECEIVE_BUF_SIZE \
> > > > (4096 + (REDS_AGENT_WINDOW_SIZE +
> > > > REDS_NUM_INTERNAL_AGENT_MESSAGES) *
> > > > SPICE_AGENT_MAX_DATA_SIZE)
> > > >
> > > > +G_DEFINE_TYPE(InputsChannel, inputs_channel,
> > > > RED_TYPE_CHANNEL)
> > > > +
> > > > +#define CHANNEL_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE((o),
> > > > TYPE_INPUTS_CHANNEL, InputsChannelPrivate))
> > > > +
> > > > +struct InputsChannelPrivate
> > > > +{
> > > > + uint8_t recv_buf[RECEIVE_BUF_SIZE];
> > > > + VDAgentMouseState mouse_state;
> > > > + int src_during_migrate;
> > > > + SpiceTimer *key_modifiers_timer;
> > > > +
> > > > + SpiceKbdInstance *keyboard;
> > > > + SpiceMouseInstance *mouse;
> > > > + SpiceTabletInstance *tablet;
> > > > +};
> > > > +
> > > > +
> > > > +static void
> > > > +inputs_channel_get_property(GObject *object,
> > > > + guint property_id,
> > > > + GValue *value,
> > > > + GParamSpec *pspec)
> > > > +{
> > > > + switch (property_id)
> > > > + {
> > > > + default:
> > > > + G_OBJECT_WARN_INVALID_PROPERTY_ID(object,
> > > > property_id,
> > > > pspec);
> > > > + }
> > > > +}
> > > > +
> > > > +static void
> > > > +inputs_channel_set_property(GObject *object,
> > > > + guint property_id,
> > > > + const GValue *value,
> > > > + GParamSpec *pspec)
> > > > +{
> > > > + switch (property_id)
> > > > + {
> > > > + default:
> > > > + G_OBJECT_WARN_INVALID_PROPERTY_ID(object,
> > > > property_id,
> > > > pspec);
> > > > + }
> > > > +}
> > > > +
> > >
> > > I noted there are these empty function here and in different
> > > file.
> > > Don't GObject provide some default functions like these?
> >
> > In general, GObject is a pain and has a lot of boilerplate. So I
> > often
> > use a GObject "generator" application that creates a bunch of
> > function
> > skeletons. In this case I simply forgot to delete the ones I
> > didn't
> > need.
> >
>
> I personally don't like that much generators if they generate too
> much code as you'll have to maintain lot of code.
> In this case for instance if the next version of GObject decide to
> change the way to implement properties you'll have to update all
> that code.
> Just to make clear I don't consider programs like bison/flex/gperf
> as generators as you don't expect to maintain their output but
> you just use them in the chain.
>
> > >
> > > >
> > > > +static void inputs_connect(RedChannel *channel, RedClient
> > > > *client,
> > > > + RedsStream *stream, int migration,
> > > > + int num_common_caps, uint32_t
> > > > *common_caps,
> > > > + int num_caps, uint32_t *caps);
> > > > +static void inputs_migrate(RedChannelClient *rcc);
> > > > +static void key_modifiers_sender(void *opaque);
> > > > +
> > > > +static void
> > > > +inputs_channel_constructed(GObject *object)
> > > > +{
> > > > + ClientCbs client_cbs = { NULL, };
> > > > + InputsChannel *self = INPUTS_CHANNEL(object);
> > > > + RedsState *reds =
> > > > red_channel_get_server(RED_CHANNEL(self));
> > > > +
> > > > + G_OBJECT_CLASS(inputs_channel_parent_class)-
> > > > > constructed(object);
> > > >
> > > > +
> > > > + client_cbs.connect = inputs_connect;
> > > > + client_cbs.migrate = inputs_migrate;
> > > > + red_channel_register_client_cbs(RED_CHANNEL(self),
> > > > &client_cbs, NULL);
> > > > +
> > > > + red_channel_set_cap(RED_CHANNEL(self),
> > > > SPICE_INPUTS_CAP_KEY_SCANCODE);
> > > > + reds_register_channel(reds, RED_CHANNEL(self));
> > > > +
> > > > + if (!(self->priv->key_modifiers_timer =
> > > > reds_core_timer_add(reds,
> > > > key_modifiers_sender, self))) {
> > > > + spice_error("key modifiers timer create failed");
> > > > + }
> > > > +}
> > > > +
> > > > +static void
> > > > +inputs_channel_init(InputsChannel *self)
> > > > +{
> > > > + self->priv = CHANNEL_PRIVATE(self);
> > > > +}
> > > > +
> > > > struct SpiceKbdState {
> > > > uint8_t push_ext_type;
> > > >
> > > > @@ -105,18 +182,6 @@ RedsState*
> > > > spice_tablet_state_get_server(SpiceTabletState *st)
> > > > return st->reds;
> > > > }
> > > >
> > > > -struct InputsChannel {
> > > > - RedChannel base;
> > > > - uint8_t recv_buf[RECEIVE_BUF_SIZE];
> > > > - VDAgentMouseState mouse_state;
> > > > - int src_during_migrate;
> > > > - SpiceTimer *key_modifiers_timer;
> > > > -
> > > > - SpiceKbdInstance *keyboard;
> > > > - SpiceMouseInstance *mouse;
> > > > - SpiceTabletInstance *tablet;
> > > > -};
> > > > -
> > >
> > > Couldn't we move new function after this declaration to reduce
> > > patch
> > > size?
> >
> > Yeah, I'll try to reduce the diff a bit more.
> >
> > >
> > > >
> > > > typedef struct RedKeyModifiersPipeItem {
> > > > RedPipeItem base;
> > > > uint8_t modifiers;
> > >
> > > ... omissis ...
> > >
>
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel