Re: [Spice-devel] [win32/vd_agent 1/4] construct WDDMMonitorConfigEscape also with NULL DisplayMode
>
> Adding possibility to construct WDDMMonitorConfigEscape object
> with NULL parameter of DisplayMode. In this case all the fields
> depending on DisplayMode are set to zero.
>
> Signed-off-by: Yuri Benditovich
Acked-by: Frediano Ziglio
> ---
> vdagent/display_configuration.cpp | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/vdagent/display_configuration.cpp
> b/vdagent/display_configuration.cpp
> index cdbbe23..bb2fb80 100644
> --- a/vdagent/display_configuration.cpp
> +++ b/vdagent/display_configuration.cpp
> @@ -242,10 +242,10 @@ struct WDDMMonitorConfigEscape {
> {
> _ioctl = QXL_ESCAPE_MONITOR_CONFIG;
> _head.id = _head.surface_id = 0;
> -_head.x = mode->get_pos_x();
> -_head.y = mode->get_pos_y();
> -_head.width = mode->get_width();
> -_head.height = mode->get_height();
> +_head.x = mode ? mode->get_pos_x() : 0;
> +_head.y = mode ? mode->get_pos_y() : 0;
> +_head.width = mode ? mode->get_width() : 0;
> +_head.height = mode ? mode->get_height() : 0;
> }
> uint32_t_ioctl;
> QXLHead _head;
Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [win32/vd_agent 2/4] add to CCD object reference to parent WDDM object
>
> Adding to CCD object reference to WDDM interface that includes it.
> This allows to use WDDM methods from CCD methods.
>
This is just spaghetti code. CCD was added to deal with a specific
Windows API, if now it calls WDDMInterface it does not make any sense.
Would be better to just send the escape from WDDMInterface::set_monitor_state
in case the state became detached.
> Signed-off-by: Yuri Benditovich
> ---
> vdagent/display_configuration.cpp | 4 +++-
> vdagent/display_configuration.h | 5 -
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/vdagent/display_configuration.cpp
> b/vdagent/display_configuration.cpp
> index bb2fb80..f535a38 100644
> --- a/vdagent/display_configuration.cpp
> +++ b/vdagent/display_configuration.cpp
> @@ -416,6 +416,7 @@ WDDMInterface::WDDMInterface()
> , _pfnEscape(NULL)
> , _pfnOpen_adapter_device_name(NULL)
> , _pfnOpen_adapter_gdi_name(NULL)
> +, _ccd(*this)
> {
> LONG error(0);
> //Can we find the D3D calls we need?
> @@ -679,7 +680,7 @@ bool WDDMInterface::escape(LPCTSTR device_name, void*
> data, UINT size_data)
> return NT_SUCCESS(status);
> }
>
> -CCD::CCD()
> +CCD::CCD(WDDMInterface& wddm)
> :_numPathElements(0)
> ,_numModeElements(0)
> ,_pPathInfo(NULL)
> @@ -690,6 +691,7 @@ CCD::CCD()
> ,_pfnSetDisplayConfig(NULL)
> ,_primary_detached(false)
> ,_path_state(PATH_UPDATED)
> +,_wddm(wddm)
> {
> load_api();
> get_config_buffers();
> diff --git a/vdagent/display_configuration.h
> b/vdagent/display_configuration.h
> index 7b5578e..81fabfe 100644
> --- a/vdagent/display_configuration.h
> +++ b/vdagent/display_configuration.h
> @@ -48,9 +48,11 @@ typedef LONG(APIENTRY* PQUERYDISPLAYCONFIG)(UINT32,
> UINT32*, DISPLAYCONFIG_PATH_
> typedef LONG(APIENTRY* PSETDISPLAYCONFIG)(UINT32, DISPLAYCONFIG_PATH_INFO*,
> UINT32,
>DISPLAYCONFIG_MODE_INFO*, UINT32);
>
> +class WDDMInterface;
> +
> class CCD {
> public:
> -CCD();
> +CCD(WDDMInterface& wddm);
> ~CCD();
>
> bool query_display_config();
> @@ -87,6 +89,7 @@ private:
>
> bool _primary_detached;
> PATH_STATE _path_state;
> +WDDMInterface& _wddm;
> };
>
> class DisplayMode;
Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk 3/3] clipboard: invalidate targets request when needed
Hi,
On Wed, Mar 6, 2019 at 7:11 PM Marc-André Lureau
wrote:
>
> Hi
>
> On Thu, Feb 28, 2019 at 8:12 PM Jakub Janků wrote:
> >
> > Targets request is no longer relevant when
> > clipboard owner changes since the retrieved targets
> > will be outdated.
> >
> > When the request is no longer relevant,
> > invalidate it by pointing its weak ref to NULL.
> > As a consequence, free_weak_ref() call returns NULL
> > and clipboard_get_targets() exits.
>
> Why not simply check if user_data == last_targets_request?
This surely looks simpler,
thanks :)
Jakub
>
> What does nullify_weak_ref() really helps with?
>
> >
> > Signed-off-by: Jakub Janků
> > ---
> >
> > This addresses the same issue that
> > was present in spice-vdagent and is already fixed.
> >
> > ---
> > src/spice-gtk-session.c | 39 +--
> > 1 file changed, 29 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > index 018cb4b..037547a 100644
> > --- a/src/spice-gtk-session.c
> > +++ b/src/spice-gtk-session.c
> > @@ -59,6 +59,7 @@ struct _SpiceGtkSessionPrivate {
> > gbooleanclip_hasdata[CLIPBOARD_LAST];
> > gbooleanclip_grabbed[CLIPBOARD_LAST];
> > gbooleanclipboard_by_guest[CLIPBOARD_LAST];
> > +GWeakRef*last_targets_request[CLIPBOARD_LAST];
> > /* auto-usbredir related */
> > gbooleanauto_usbredir_enable;
> > int auto_usbredir_reqs;
> > @@ -537,6 +538,16 @@ static gpointer free_weak_ref(gpointer data)
> > return object;
> > }
> >
> > +static void nullify_weak_ref(GWeakRef **weak_ref_p)
> > +{
> > +gpointer null_object = NULL;
> > +
> > +if (*weak_ref_p) {
> > +g_weak_ref_set(*weak_ref_p, null_object);
> > +*weak_ref_p = NULL;
> > +}
> > +}
> > +
> > static void clipboard_get_targets(GtkClipboard *clipboard,
> >GdkAtom *atoms,
> >gint n_atoms,
> > @@ -551,23 +562,25 @@ static void clipboard_get_targets(GtkClipboard
> > *clipboard,
> >
> > g_return_if_fail(SPICE_IS_GTK_SESSION(self));
> >
> > -if (atoms == NULL) {
> > -SPICE_DEBUG("Retrieving the clipboard data has failed");
> > -return;
> > -}
> > -
> > SpiceGtkSessionPrivate *s = self->priv;
> > guint32 types[SPICE_N_ELEMENTS(atom2agent)] = { 0 };
> > gint num_types;
> > int a;
> > int selection;
> >
> > -if (s->main == NULL)
> > -return;
> > -
> > selection = get_selection_from_clipboard(s, clipboard);
> > g_return_if_fail(selection != -1);
> >
> > +s->last_targets_request[selection] = NULL;
> > +
> > +if (atoms == NULL) {
> > +SPICE_DEBUG("Retrieving the clipboard data has failed");
> > +return;
> > +}
> > +
> > +if (s->main == NULL)
> > +return;
> > +
> > if (s->clip_grabbed[selection]) {
> > SPICE_DEBUG("Clipboard is already grabbed, ignoring %d atoms",
> > n_atoms);
> > return;
> > @@ -652,6 +665,8 @@ static void clipboard_owner_change(GtkClipboard
> > *clipboard,
> > return;
> > }
> >
> > +nullify_weak_ref(>last_targets_request[selection]);
> > +
> > /* In case we sent a grab to the agent, we need to release it now as
> > * previous clipboard data should not be reachable anymore */
> > if (s->clip_grabbed[selection]) {
> > @@ -690,9 +705,11 @@ static void clipboard_owner_change(GtkClipboard
> > *clipboard,
> > #endif
> >
> > s->clip_hasdata[selection] = TRUE;
> > -if (s->auto_clipboard_enable && !read_only(self))
> > +if (s->auto_clipboard_enable && !read_only(self)) {
> > +s->last_targets_request[selection] = get_weak_ref(self);
> > gtk_clipboard_request_targets(clipboard, clipboard_get_targets,
> > - get_weak_ref(self));
> > + s->last_targets_request[selection]);
> > +}
> > }
> >
> > typedef struct
> > @@ -866,6 +883,8 @@ static gboolean clipboard_grab(SpiceMainChannel *main,
> > guint selection,
> > return TRUE;
> > }
> >
> > +nullify_weak_ref(>last_targets_request[selection]);
> > +
> > if (!gtk_clipboard_set_with_owner(cb,
> >targets,
> >num_targets,
> > --
> > 2.20.1
> >
> > ___
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
>
>
> --
> Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-gtk 1/3] clipboard: accept grab only from the side with keyboard focus
Hi, thanks for having a look! On Wed, Mar 6, 2019 at 6:42 PM Marc-André Lureau wrote: > > Hi > > On Thu, Feb 28, 2019 at 8:12 PM Jakub Janků wrote: > > > > If two grab messages in opposite directions "meet" on their way > > to their destinations, we end up in a state when both spice-gtk > > and spice-vdagent think that the other component can provide > > clipboard data. As a consequence of this, neither of them can. > > > > This is a bug in the spice-protocol. To mitigate the issue, > > With such as statement, you should provide detailed analysis, and > strong arguments for workarounds. Depends on what you consider to be "detailed analysis". Since you correctly restated the problem with your own words, it seems like my rather short description served its purpose :) Also please note that I've tried to provide a more detailed analysis of the issue reported by James Harvey earlier in [0] [0] https://lists.freedesktop.org/archives/spice-devel/2019-January/047614.html As for the arguments, I've actually expressed my main argument in the cover letter, have you read it? "Intention of these patches is to make spice-gtk behave reasonably well with older agents." I would like to have this patch reviewed with this in mind. So this patch NOT trying to be a final solution to the problem. If you look at the race condition from the standpoint of spice-gtk, the situation looks the following: 1) spice-gtk sends grab 2) spice-gtk receives grab You can't tell if it's a race or not. The normal behaviour can look the same. Now, if you consider fix in spice-gtk only (to make older vdagents play nicely with new spice-gtk), I see these possible mitigations: a) measure time between 1) and 2) and discard one grab if the elapsed time is too short: this solution is rather empirical and seems unreliable to me b) accept grab only from one side at a moment - that is what this patch does > > I think you are describing this conflicting situation: (all messages > are asynchronous here, A & B are either client or remote end > interchangeably) > > A->B: grab > B->A: grab > A: handle B grab > B: handle A grab > > Both A think the other end has the grab... > > If we would instead explicitly release the grab, none would have it, > which could be considered an improvement: Yes, a slight improvement. However, the original grabs both in client and guest would be lost, which is unwanted. > > A->B: grab > B->A: grab > A: handle B grab > A->B: release > B: handle A grab > B->A: release > B: handle A release > A: handle B release > > Instead, we should probably have a priority for who wins. I suppose it > should be the client. Not necessarily. Consider James' case: if we make the client win, the clipboard manager in the client would take over the clipboard in the guest. The clipboard manager filters out some of the targets and additionally, the "marching ants" in Excel would disappear. >But how to distinguish the conflict case with > the normal case? Perhaps an incremental counter (age/generation, > whatever you call it) would be enough. This would need a protocol change. So given my note in the cover letter, this comment is quite unrelated to this patch. I would prefer to discuss the protocol change later. > > Did you thought about something along those lines? Would you be able > to come up with protocol changes for that? > > At this point, I think we could use some nice sequence diagrams in the spec! > > > > accept grab only from the side that currently has keyboard focus, > > this means: > > (1) spice-gtk has focus ==> > > * accept grabs from vdagent, > > * ignore grabs from other apps running in the host > > I have a hard time understanding why keyboard focus is related to > clipboard. clipboard can be interacted with mouse only, or > independently from any user input. Or from multiple inputs. I try to > fit the keyboard input this into clipboard interaction issues, and I > don't think that helps much. If spice-gtk has keyboard focus, the user interacts with the guest system. If it doesn't, user interacts with the client system. Sure, the clipboard can change without user's input. E.g. with some automation systems, as you pointer out earlier. However, this is not the usual use case. And with this patch, I'm trying to fix the most common scenario to provide a better default experience. > > > (2) spice-gtk doesn't have focus ==> > > * accept grabs from other apps running in the host > > * ignore grabs from vdagent > > > > The check in clipboard_owner_change() is X11-specific, > > because on Wayland, spice-gtk is first notified about the > > owner-change once it gains focus. > > > > The check in clipboard_grab() can be generic. > > Note that on Wayland, calling gtk_clipboard_set_with_owner() > > while not having focus doesn't have any effect anyway, > > only focused clients can set clipboard. > > > > With this patch, the race can still occur around the time > > when focus changes (rare, but possible), on X11 as well
Re: [Spice-devel] [PATCH spice-common 9/9] codegen: Rename --prefix parameter to --suffix
> > On Sun, Mar 03, 2019 at 07:10:30PM +, Frediano Ziglio wrote: > > The option is used to add a suffix to public functions, not a > > prefix. > > Sometimes it's a suffix to a prefix, sometimes it's indeed just a > suffix. So your change makes sense. > I cannot find any place where is used as a "suffix to a prefix", it's always appended to a partial function name. > > Currently the option is not used (it was used to generate protocol > > 1 code). > > Why not remove it? It's inconsistent with the commit saying ptr_size is > unused so it can be removed. > Yes, I think 7/9 needs more explanations. It does not hurt to have this option. > Acked-by: Christophe Fergeau > > Christophe > Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-common 7/9] codegen: Remove support for --ptrsize
>
> Acked-by: Christophe Fergeau
>
Thanks, looking at reply to 9/9 however I think one thing is not clear from
what I wrote in the commit message.
The reason is not only that was used in protocol 1 only and now is not used
but also that it was wrong from the beginning and useless. I would keep it
if I found it useful.
> On Sun, Mar 03, 2019 at 07:10:28PM +, Frediano Ziglio wrote:
> > This option was used in protocol 1 to generate 64 bit pointers.
> > A pointer in the protocol is an offset in the current message.
> > This allows the possibility to have messages with pointers with
> > more than 4GB. This feature was removed and not used in protocol 2.
> > The reason is that messages more than 4GB would cause:
Maybe this could be changed in
"The reason this feature was correctly removed in protocol 2 is that
having 64 bit pointers in the protocol would require messages larger
than 4GB which would cause:"
> > - huge latency as a single message would take more than 4 seconds
> > to be send in a 10Gb connection;
> > - huge memory requirements.
> >
> > Signed-off-by: Frediano Ziglio
> > ---
> > common/marshaller.c | 14 +++---
> > common/marshaller.h | 2 +-
> > python_modules/marshal.py | 2 +-
> > python_modules/ptypes.py | 18 +++---
> > spice_codegen.py | 4
> > 5 files changed, 8 insertions(+), 32 deletions(-)
> >
> > diff --git a/common/marshaller.c b/common/marshaller.c
> > index 4379aa6..c77129b 100644
> > --- a/common/marshaller.c
> > +++ b/common/marshaller.c
> > @@ -96,7 +96,6 @@ typedef struct SpiceMarshallerData SpiceMarshallerData;
> > typedef struct {
> > SpiceMarshaller *marshaller;
> > int item_nr;
> > -int is_64bit;
> > size_t offset;
> > } MarshallerRef;
> >
> > @@ -419,13 +418,13 @@ SpiceMarshaller
> > *spice_marshaller_get_submarshaller(SpiceMarshaller *m)
> > return m2;
> > }
> >
> > -SpiceMarshaller *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller
> > *m, int is_64bit)
> > +SpiceMarshaller *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller
> > *m)
> > {
> > SpiceMarshaller *m2;
> > uint8_t *p;
> > int size;
> >
> > -size = is_64bit ? 8 : 4;
> > +size = 4;
> >
> > p = spice_marshaller_reserve_space(m, size);
> > memset(p, 0, size);
> > @@ -433,7 +432,6 @@ SpiceMarshaller
> > *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller *m, int
> > m2->pointer_ref.marshaller = m;
> > m2->pointer_ref.item_nr = m->n_items - 1;
> > m2->pointer_ref.offset = m->items[m->n_items - 1].len - size;
> > -m2->pointer_ref.is_64bit = is_64bit;
> >
> > return m2;
> > }
> > @@ -538,13 +536,7 @@ void spice_marshaller_flush(SpiceMarshaller *m)
> > for (m2 = m; m2 != NULL; m2 = m2->next) {
> > if (m2->pointer_ref.marshaller != NULL && m2->total_size > 0) {
> > ptr_pos = lookup_ref(>pointer_ref);
> > -if (m2->pointer_ref.is_64bit) {
> > -write_uint64(ptr_pos,
> > - spice_marshaller_get_offset(m2));
> > -} else {
> > -write_uint32(ptr_pos,
> > - spice_marshaller_get_offset(m2));
> > -}
> > +write_uint32(ptr_pos, spice_marshaller_get_offset(m2));
> > }
> > }
> > }
> > diff --git a/common/marshaller.h b/common/marshaller.h
> > index 041d16e..a631c85 100644
> > --- a/common/marshaller.h
> > +++ b/common/marshaller.h
> > @@ -53,7 +53,7 @@ size_t spice_marshaller_get_offset(SpiceMarshaller *m);
> > size_t spice_marshaller_get_size(SpiceMarshaller *m);
> > size_t spice_marshaller_get_total_size(SpiceMarshaller *m);
> > SpiceMarshaller *spice_marshaller_get_submarshaller(SpiceMarshaller *m);
> > -SpiceMarshaller *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller
> > *m, int is_64bit);
> > +SpiceMarshaller *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller
> > *m);
> > int spice_marshaller_fill_iovec(SpiceMarshaller *m, struct iovec *vec,
> > int n_vec, size_t skip_bytes);
> > void *spice_marshaller_add_uint64(SpiceMarshaller *m, uint64_t v);
> > diff --git a/python_modules/marshal.py b/python_modules/marshal.py
> > index 4e98993..74f3a54 100644
> > --- a/python_modules/marshal.py
> > +++ b/python_modules/marshal.py
> > @@ -234,7 +234,7 @@ def write_array_marshaller(writer, member, array,
> > container_src, scope):
> > def write_pointer_marshaller(writer, member, src):
> > t = member.member_type
> > ptr_func = write_marshal_ptr_function(writer, t.target_type)
> > -submarshaller = "spice_marshaller_get_ptr_submarshaller(m, %d)" % (1
> > if member.get_fixed_nw_size() == 8 else 0)
> > +submarshaller = "spice_marshaller_get_ptr_submarshaller(m)"
> > if member.has_attr("marshall"):
> > rest_args = ""
> > if t.target_type.is_array():
> > diff --git a/python_modules/ptypes.py
Re: [Spice-devel] [PATCH spice-common 9/9] codegen: Rename --prefix parameter to --suffix
On Sun, Mar 03, 2019 at 07:10:30PM +, Frediano Ziglio wrote: > The option is used to add a suffix to public functions, not a > prefix. Sometimes it's a suffix to a prefix, sometimes it's indeed just a suffix. So your change makes sense. > Currently the option is not used (it was used to generate protocol > 1 code). Why not remove it? It's inconsistent with the commit saying ptr_size is unused so it can be removed. Acked-by: Christophe Fergeau Christophe signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-common 7/9] codegen: Remove support for --ptrsize
Acked-by: Christophe Fergeau
On Sun, Mar 03, 2019 at 07:10:28PM +, Frediano Ziglio wrote:
> This option was used in protocol 1 to generate 64 bit pointers.
> A pointer in the protocol is an offset in the current message.
> This allows the possibility to have messages with pointers with
> more than 4GB. This feature was removed and not used in protocol 2.
> The reason is that messages more than 4GB would cause:
> - huge latency as a single message would take more than 4 seconds
> to be send in a 10Gb connection;
> - huge memory requirements.
>
> Signed-off-by: Frediano Ziglio
> ---
> common/marshaller.c | 14 +++---
> common/marshaller.h | 2 +-
> python_modules/marshal.py | 2 +-
> python_modules/ptypes.py | 18 +++---
> spice_codegen.py | 4
> 5 files changed, 8 insertions(+), 32 deletions(-)
>
> diff --git a/common/marshaller.c b/common/marshaller.c
> index 4379aa6..c77129b 100644
> --- a/common/marshaller.c
> +++ b/common/marshaller.c
> @@ -96,7 +96,6 @@ typedef struct SpiceMarshallerData SpiceMarshallerData;
> typedef struct {
> SpiceMarshaller *marshaller;
> int item_nr;
> -int is_64bit;
> size_t offset;
> } MarshallerRef;
>
> @@ -419,13 +418,13 @@ SpiceMarshaller
> *spice_marshaller_get_submarshaller(SpiceMarshaller *m)
> return m2;
> }
>
> -SpiceMarshaller *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller *m,
> int is_64bit)
> +SpiceMarshaller *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller *m)
> {
> SpiceMarshaller *m2;
> uint8_t *p;
> int size;
>
> -size = is_64bit ? 8 : 4;
> +size = 4;
>
> p = spice_marshaller_reserve_space(m, size);
> memset(p, 0, size);
> @@ -433,7 +432,6 @@ SpiceMarshaller
> *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller *m, int
> m2->pointer_ref.marshaller = m;
> m2->pointer_ref.item_nr = m->n_items - 1;
> m2->pointer_ref.offset = m->items[m->n_items - 1].len - size;
> -m2->pointer_ref.is_64bit = is_64bit;
>
> return m2;
> }
> @@ -538,13 +536,7 @@ void spice_marshaller_flush(SpiceMarshaller *m)
> for (m2 = m; m2 != NULL; m2 = m2->next) {
> if (m2->pointer_ref.marshaller != NULL && m2->total_size > 0) {
> ptr_pos = lookup_ref(>pointer_ref);
> -if (m2->pointer_ref.is_64bit) {
> -write_uint64(ptr_pos,
> - spice_marshaller_get_offset(m2));
> -} else {
> -write_uint32(ptr_pos,
> - spice_marshaller_get_offset(m2));
> -}
> +write_uint32(ptr_pos, spice_marshaller_get_offset(m2));
> }
> }
> }
> diff --git a/common/marshaller.h b/common/marshaller.h
> index 041d16e..a631c85 100644
> --- a/common/marshaller.h
> +++ b/common/marshaller.h
> @@ -53,7 +53,7 @@ size_t spice_marshaller_get_offset(SpiceMarshaller *m);
> size_t spice_marshaller_get_size(SpiceMarshaller *m);
> size_t spice_marshaller_get_total_size(SpiceMarshaller *m);
> SpiceMarshaller *spice_marshaller_get_submarshaller(SpiceMarshaller *m);
> -SpiceMarshaller *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller *m,
> int is_64bit);
> +SpiceMarshaller *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller *m);
> int spice_marshaller_fill_iovec(SpiceMarshaller *m, struct iovec *vec,
> int n_vec, size_t skip_bytes);
> void *spice_marshaller_add_uint64(SpiceMarshaller *m, uint64_t v);
> diff --git a/python_modules/marshal.py b/python_modules/marshal.py
> index 4e98993..74f3a54 100644
> --- a/python_modules/marshal.py
> +++ b/python_modules/marshal.py
> @@ -234,7 +234,7 @@ def write_array_marshaller(writer, member, array,
> container_src, scope):
> def write_pointer_marshaller(writer, member, src):
> t = member.member_type
> ptr_func = write_marshal_ptr_function(writer, t.target_type)
> -submarshaller = "spice_marshaller_get_ptr_submarshaller(m, %d)" % (1 if
> member.get_fixed_nw_size() == 8 else 0)
> +submarshaller = "spice_marshaller_get_ptr_submarshaller(m)"
> if member.has_attr("marshall"):
> rest_args = ""
> if t.target_type.is_array():
> diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
> index 64c198e..bd5f542 100644
> --- a/python_modules/ptypes.py
> +++ b/python_modules/ptypes.py
> @@ -4,8 +4,6 @@ import types
> _types_by_name = {}
> _types = []
>
> -default_pointer_size = 4
> -
> def type_exists(name):
> return name in _types_by_name
>
> @@ -512,7 +510,6 @@ class PointerType(Type):
> Type.__init__(self)
> self.name = None
> self.target_type = target_type
> -self.pointer_size = default_pointer_size
>
> def __str__(self):
> return "%s*" % (str(self.target_type))
> @@ -521,9 +518,6 @@ class PointerType(Type):
> self.target_type = self.target_type.resolve()
> return self
>
> -def
Re: [Spice-devel] [PATCH spice-common 6/9] Generate automatically most C message declarations
Acked-by: Christophe Fergeau
only one caveat is that some of the structs are getting renamed to
equivalent ones (SpiceMsgDisplayDrawBlend/SpiceMsgDisplayDrawCopy,
SpiceMsgcMainAgentStart/SpiceMsgcMainAgentTokens and a few more), which
is fine because they are typedef'ed to the other type anyway
I guess it's worth mentioned in the commit log.
Reviewed-by: Christophe Fergeau
Christophe
On Sun, Mar 03, 2019 at 07:10:27PM +, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio
> ---
> common/messages.h | 495 +-
> spice.proto | 192 +-
> 2 files changed, 102 insertions(+), 585 deletions(-)
>
> diff --git a/common/messages.h b/common/messages.h
> index 36ee59d..5cda1d1 100644
> --- a/common/messages.h
> +++ b/common/messages.h
> @@ -42,10 +42,6 @@
>
> SPICE_BEGIN_DECLS
>
> -typedef struct SpiceMsgData {
> -uint8_t data[0];
> -} SpiceMsgData;
> -
> typedef struct SpiceMsgCompressedData {
> uint8_t type;
> uint32_t uncompressed_size;
> @@ -53,438 +49,8 @@ typedef struct SpiceMsgCompressedData {
> uint8_t *compressed_data;
> } SpiceMsgCompressedData;
>
> -typedef struct SpiceMsgEmpty {
> -uint8_t padding;
> -} SpiceMsgEmpty;
> -
> -typedef struct SpiceMsgInputsInit {
> -uint32_t keyboard_modifiers;
> -} SpiceMsgInputsInit;
> -
> -typedef struct SpiceMsgInputsKeyModifiers {
> -uint32_t modifiers;
> -} SpiceMsgInputsKeyModifiers;
> -
> -typedef struct SpiceMsgMainMultiMediaTime {
> -uint32_t time;
> -} SpiceMsgMainMultiMediaTime;
> -
> -typedef struct SpiceMigrationDstInfo {
> -uint16_t port;
> -uint16_t sport;
> -uint32_t host_size;
> -uint8_t *host_data;
> -uint32_t cert_subject_size;
> -uint8_t *cert_subject_data;
> -} SpiceMigrationDstInfo;
> -
> -typedef struct SpiceMsgMainMigrationBegin {
> -SpiceMigrationDstInfo dst_info;
> -} SpiceMsgMainMigrationBegin;
> -
> -typedef struct SpiceMsgMainMigrateBeginSeamless {
> -SpiceMigrationDstInfo dst_info;
> -uint32_t src_mig_version;
> -} SpiceMsgMainMigrateBeginSeamless;
> -
> -typedef struct SpiceMsgcMainMigrateDstDoSeamless {
> -uint32_t src_version;
> -} SpiceMsgcMainMigrateDstDoSeamless;
> -
> -typedef struct SpiceMsgMainMigrationSwitchHost {
> -uint16_t port;
> -uint16_t sport;
> -uint32_t host_size;
> -uint8_t *host_data;
> -uint32_t cert_subject_size;
> -uint8_t *cert_subject_data;
> -} SpiceMsgMainMigrationSwitchHost;
> -
> -
> -typedef struct SpiceMsgMigrate {
> -uint32_t flags;
> -} SpiceMsgMigrate;
> -
> -typedef struct SpiceResourceID {
> -uint8_t type;
> -uint64_t id;
> -} SpiceResourceID;
> -
> -typedef struct SpiceResourceList {
> -uint16_t count;
> -SpiceResourceID resources[0];
> -} SpiceResourceList;
> -
> -typedef struct SpiceMsgSetAck {
> -uint32_t generation;
> -uint32_t window;
> -} SpiceMsgSetAck;
> -
> -typedef struct SpiceMsgcAckSync {
> - uint32_t generation;
> -} SpiceMsgcAckSync;
> -
> -typedef struct SpiceWaitForChannel {
> -uint8_t channel_type;
> -uint8_t channel_id;
> -uint64_t message_serial;
> -} SpiceWaitForChannel;
> -
> -typedef struct SpiceMsgWaitForChannels {
> -uint8_t wait_count;
> -SpiceWaitForChannel wait_list[0];
> -} SpiceMsgWaitForChannels;
> -
> -typedef struct SpiceChannelId {
> -uint8_t type;
> -uint8_t id;
> -} SpiceChannelId;
> -
> -typedef struct SpiceMsgMainInit {
> -uint32_t session_id;
> -uint32_t display_channels_hint;
> -uint32_t supported_mouse_modes;
> -uint32_t current_mouse_mode;
> -uint32_t agent_connected;
> -uint32_t agent_tokens;
> -uint32_t multi_media_time;
> -uint32_t ram_hint;
> -} SpiceMsgMainInit;
> -
> -typedef struct SpiceMsgDisconnect {
> -uint64_t time_stamp;
> -uint32_t reason; // SPICE_ERR_?
> -} SpiceMsgDisconnect;
> -
> -typedef struct SpiceMsgNotify {
> -uint64_t time_stamp;
> -uint32_t severity;
> -uint32_t visibilty;
> -uint32_t what;
> -uint32_t message_len;
> -uint8_t message[0];
> -} SpiceMsgNotify;
> -
> -typedef struct SpiceMsgChannels {
> -uint32_t num_of_channels;
> -SpiceChannelId channels[0];
> -} SpiceMsgChannels;
> -
> -typedef struct SpiceMsgMainName {
> -uint32_t name_len;
> -uint8_t name[0];
> -} SpiceMsgMainName;
> -
> -typedef struct SpiceMsgMainUuid {
> -uint8_t uuid[16];
> -} SpiceMsgMainUuid;
> -
> -typedef struct SpiceMsgMainMouseMode {
> -uint32_t supported_modes;
> -uint32_t current_mode;
> -} SpiceMsgMainMouseMode;
> -
> -typedef struct SpiceMsgPing {
> -uint32_t id;
> -uint64_t timestamp;
> -void *data;
> -uint32_t data_len;
> -} SpiceMsgPing;
> -
> -typedef struct SpiceMsgMainAgentDisconnect {
> -uint32_t error_code; // SPICE_ERR_?
> -} SpiceMsgMainAgentDisconnect;
> -
> #define SPICE_AGENT_MAX_DATA_SIZE 2048
>
> -typedef struct SpiceMsgMainAgentTokens {
> -uint32_t num_tokens;
> -}
Re: [Spice-devel] [PATCH spice-common 5/9] Allow to generate C declarations for spice.proto
On Sun, Mar 03, 2019 at 07:10:26PM +, Frediano Ziglio wrote:
> Generate and include C declarations.
> Next patch will use this facility.
Since none of the spice.proto types are decorated with @declare, adding
the #include in messages.h won't have any bad consequences
>
> Signed-off-by: Frediano Ziglio
> ---
> common/Makefile.am | 6 --
> common/meson.build | 10 +-
> common/messages.h | 2 ++
> 3 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/common/Makefile.am b/common/Makefile.am
> index 3da5bad..411831b 100644
> --- a/common/Makefile.am
> +++ b/common/Makefile.am
> @@ -109,8 +109,9 @@ MARSHALLERS_DEPS =
> \
>
> # Note despite being autogenerated these are not part of CLEANFILES, they are
> # actually a part of EXTRA_DIST, to avoid the need for pyparser by end users
> -generated_client_demarshallers.c: $(top_srcdir)/spice.proto
> $(MARSHALLERS_DEPS)
> - $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
> --generate-demarshallers --client --include common/messages.h $< $@ >/dev/null
> +generated_client_demarshallers.c generated_messages.h:
> $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
> + $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
> --generate-demarshallers --client --include common/messages.h \
> + --generated-declaration-file generated_messages.h $< $@ >/dev/null
>
> generated_client_marshallers.c generated_client_marshallers.h:
> $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS)
> $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
> --generate-marshallers -P --include client_marshallers.h --client \
> @@ -135,6 +136,7 @@ EXTRA_DIST = \
> quic_family_tmpl.c \
> quic_tmpl.c \
> snd_codec.h \
> + generated_messages.h\
Unrelated to this patch, but I wonder why snd_codec.h is listed there.
I'd put generated_messages.h close to the $(CLIENT_MARSHALLERS) and
$(SERVER_MARSHALLERS) variables.
Reviewed-by: Christophe Fergeau
> $(NULL)
>
> -include $(top_srcdir)/git.mk
> diff --git a/common/meson.build b/common/meson.build
> index 9575568..2d769f2 100644
> --- a/common/meson.build
> +++ b/common/meson.build
> @@ -64,7 +64,15 @@ spice_common_dep = declare_dependency(link_with :
> spice_common_lib,
> #
> if spice_common_generate_client_code
>targets = [
> -['client_demarshallers', spice_proto,
> 'generated_client_demarshallers.c', ['--generate-demarshallers', '--client',
> '--include', 'common/messages.h', '@INPUT@', '@OUTPUT@']],
> +['client_demarshallers', spice_proto,
> + ['generated_client_demarshallers.c', 'generated_messages.h'],
> + ['--generate-demarshallers',
> +'--client',
> +'--include', 'common/messages.h',
> +'--generated-declaration-file', '@OUTPUT1@',
> +'@INPUT@', '@OUTPUT0@'
> + ]
> +],
> ['client_marshallers', spice_proto,
>['generated_client_marshallers.c', 'generated_client_marshallers.h'],
>['--generate-marshallers',
> diff --git a/common/messages.h b/common/messages.h
> index f740a8c..36ee59d 100644
> --- a/common/messages.h
> +++ b/common/messages.h
> @@ -558,6 +558,8 @@ typedef struct SpiceMsgDisplayGlDraw {
> uint32_t h;
> } SpiceMsgDisplayGlDraw;
>
> +#include
> +
> SPICE_END_DECLS
>
> #endif // H_SPICE_COMMON_MESSAGES
> --
> 2.20.1
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-common 4/9] codegen: Allows to generate C declarations automatically
On Sun, Mar 03, 2019 at 07:10:25PM +, Frediano Ziglio wrote:
> Allows to specify a @declare attribute for messages and structure
> that can generate the needed C structures.
>
> Signed-off-by: Frediano Ziglio
> ---
> python_modules/ptypes.py | 64
> spice_codegen.py | 47 +
> 2 files changed, 111 insertions(+)
>
> diff --git a/python_modules/ptypes.py b/python_modules/ptypes.py
> index 7dca78d..64c198e 100644
> --- a/python_modules/ptypes.py
> +++ b/python_modules/ptypes.py
> @@ -72,6 +72,8 @@ valid_attributes=set([
> 'zero',
> # this attribute does not exist on the network, fill just structure with
> the value
> 'virtual',
> +# generate C structure declarations from protocol definition
> +'declare',
> ])
>
> attributes_with_arguments=set([
> @@ -485,6 +487,26 @@ class ArrayType(Type):
> def c_type(self):
> return self.element_type.c_type()
>
> +def generate_c_declaration(self, writer, member):
> +name = member.name
> +if member.has_attr("chunk"):
> +return writer.writeln('SpiceChunks *%s;' % name)
> +if member.has_attr("as_ptr"):
> +len_var = member.attributes["as_ptr"][0]
> +writer.writeln('uint32_t %s;' % len_var)
> +return writer.writeln('%s *%s;' % (self.c_type(), name))
> +if member.has_attr("to_ptr"): # TODO len
> +return writer.writeln('%s *%s;' % (self.c_type(), name))
> +if member.has_attr("ptr_array"): # TODO where the length is stored?
> overflow?
> +return writer.writeln('%s *%s[0];' % (self.c_type(), name))
> +if member.has_end_attr() or self.is_remaining_length(): # TODO len
> +return writer.writeln('%s %s[0];' % (self.c_type(), name))
These TODO are worrying, but only the has_end_attr() one seems to be run
at the moment, the others seem to be dead code?
Looks good otherwise,
Acked-by: Christophe Fergeau
> +if self.is_constant_length():
> +return writer.writeln('%s %s[%s];' % (self.c_type(), name,
> self.size))
> +if self.is_identifier_length():
> +return writer.writeln('%s *%s;' % (self.c_type(), name))
> +raise NotImplementedError('unknown array %s' % str(self))
> +
> class PointerType(Type):
> def __init__(self, target_type):
> Type.__init__(self)
> @@ -529,6 +551,15 @@ class PointerType(Type):
> def get_num_pointers(self):
> return 1
>
> +def generate_c_declaration(self, writer, member):
> +target_type = self.target_type
> +is_array = target_type.is_array()
> +if not is_array or target_type.is_identifier_length():
> +writer.writeln("%s *%s;" % (target_type.c_type(), member.name))
> +return
> +raise NotImplementedError('Some pointers to array declarations are
> not implemented %s' %
> +member)
> +
> class Containee:
> def __init__(self):
> self.attributes = {}
> @@ -624,6 +655,14 @@ class Member(Containee):
> names = [prefix + "_" + name for name in names]
> return names
>
> +def generate_c_declaration(self, writer):
> +if self.has_attr("zero"):
> +return
> +if self.is_pointer() or self.is_array():
> +self.member_type.generate_c_declaration(writer, self)
> +return
> +return writer.writeln("%s %s;" % (self.member_type.c_type(),
> self.name))
> +
> class SwitchCase:
> def __init__(self, values, member):
> self.values = values
> @@ -747,6 +786,17 @@ class Switch(Containee):
> names = names + c.get_pointer_names(marshalled)
> return names
>
> +def generate_c_declaration(self, writer):
> +if self.has_attr("anon") and len(self.cases) == 1:
> +self.cases[0].member.generate_c_declaration(writer)
> +return
> +writer.writeln('union {')
> +writer.indent()
> +for m in self.cases:
> +m.member.generate_c_declaration(writer)
> +writer.unindent()
> +writer.writeln('} %s;' % self.name)
> +
> class ContainerType(Type):
> def is_fixed_sizeof(self):
> for m in self.members:
> @@ -857,6 +907,20 @@ class ContainerType(Type):
>
> return member
>
> +def generate_c_declaration(self, writer):
> +if not self.has_attr('declare'):
> +return
> +name = self.c_type()
> +writer.writeln('typedef struct %s {' % name)
> +writer.indent()
> +for m in self.members:
> +m.generate_c_declaration(writer)
> +if len(self.members) == 0:
> +# make sure generated structure are not empty
> +writer.writeln("char dummy[0];")
> +writer.unindent()
> +writer.writeln('} %s;' % name).newline()
> +
> class StructType(ContainerType):
> def __init__(self, name,
Re: [Spice-devel] [PATCH spice-common 3/9] codegen: Generate headers while generating code
On Sun, Mar 03, 2019 at 07:10:24PM +, Frediano Ziglio wrote: > Python script generates code and header together however allowed > to save only one of them. > Allows to save both of them together to reduce number of time > we call Python script. > > Signed-off-by: Frediano Ziglio > --- > common/Makefile.am | 16 ++-- > common/client_marshallers.h | 2 +- > common/meson.build | 19 +++ > spice_codegen.py| 12 ++-- > 4 files changed, 32 insertions(+), 17 deletions(-) > > diff --git a/common/Makefile.am b/common/Makefile.am > index 3318009..3da5bad 100644 > --- a/common/Makefile.am > +++ b/common/Makefile.am > @@ -112,21 +112,17 @@ MARSHALLERS_DEPS = > \ > generated_client_demarshallers.c: $(top_srcdir)/spice.proto > $(MARSHALLERS_DEPS) > $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py > --generate-demarshallers --client --include common/messages.h $< $@ >/dev/null > > -generated_client_marshallers.h: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS) > - $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py > --generate-marshallers -P --client --include common/messages.h -H $< $@ > >/dev/null > - > -generated_client_marshallers.c: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS) > - $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py > --generate-marshallers -P --include client_marshallers.h --client $< $@ > >/dev/null > +generated_client_marshallers.c generated_client_marshallers.h: > $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS) > + $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py > --generate-marshallers -P --include client_marshallers.h --client \ > + --generated-header generated_client_marshallers.h $< $@ >/dev/null > > generated_server_demarshallers.c: $(top_srcdir)/spice.proto > $(MARSHALLERS_DEPS) > $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py > --generate-demarshallers --server --include common/messages.h $< $@ >/dev/null > > STRUCTS = -M String -M Rect -M Point -M DisplayBase -M Fill -M Opaque -M > Copy -M Blend -M Blackness -M Whiteness -M Invers -M Rop3 -M Stroke -M Text > -M Transparent -M AlphaBlend -M Composite > -generated_server_marshallers.c: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS) > - $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py > --generate-marshallers $(STRUCTS) --server --include common/messages.h $< $@ > >/dev/null > - > -generated_server_marshallers.h: $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS) > - $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py > --generate-marshallers $(STRUCTS) --server --include common/messages.h -H $< > $@ >/dev/null > +generated_server_marshallers.c generated_server_marshallers.h: > $(top_srcdir)/spice.proto $(MARSHALLERS_DEPS) > + $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py > --generate-marshallers $(STRUCTS) --server --include common/messages.h \ > +--generated-header generated_server_marshallers.h $< $@ >/dev/null > > EXTRA_DIST = \ > $(CLIENT_MARSHALLERS) \ > diff --git a/common/client_marshallers.h b/common/client_marshallers.h > index f082934..b67b98e 100644 > --- a/common/client_marshallers.h > +++ b/common/client_marshallers.h > @@ -21,9 +21,9 @@ > > #include > > +#include "messages.h" > #include "common/generated_client_marshallers.h" > #include "marshaller.h" > -#include "messages.h" Is this hunk intentionally in this patch? > > SPICE_BEGIN_DECLS > > diff --git a/common/meson.build b/common/meson.build > index 156297b..9575568 100644 > --- a/common/meson.build > +++ b/common/meson.build > @@ -65,8 +65,14 @@ spice_common_dep = declare_dependency(link_with : > spice_common_lib, > if spice_common_generate_client_code >targets = [ > ['client_demarshallers', spice_proto, > 'generated_client_demarshallers.c', ['--generate-demarshallers', '--client', > '--include', 'common/messages.h', '@INPUT@', '@OUTPUT@']], > -['client_marshalers', spice_proto, 'generated_client_marshallers.c', > ['--generate-marshallers', '-P', '--client', '--include', > 'client_marshallers.h', '@INPUT@', '@OUTPUT@']], > -['client_marshallers_h', spice_proto, 'generated_client_marshallers.h', > ['--generate-marshallers', '-P', '--client', '--include', > 'common/messages.h', '-H', '@INPUT@', '@OUTPUT@']], > +['client_marshallers', spice_proto, > + ['generated_client_marshallers.c', 'generated_client_marshallers.h'], > + ['--generate-marshallers', > +'-P', '--client', '--include', 'client_marshallers.h', > +'--generated-header', '@OUTPUT1@', > +'@INPUT@', '@OUTPUT0@' > + ] > +] >] > >spice_common_client_sources = [ > @@ -116,8 +122,13 @@ if spice_common_generate_server_code > >targets = [ > ['server_demarshallers', spice_proto, > 'generated_server_demarshallers.c', ['--generate-demarshallers', '--server', > '--include',
Re: [Spice-devel] [PATCH spice-common 1/9] messages: Remove fields not used by the protocol
On Sun, Mar 03, 2019 at 07:10:22PM +, Frediano Ziglio wrote:
> These fields are not used by the protocol.
Maybe make it explicit that 'not used by the protocol' means they are
not in spice.proto, nor will be used in the generated (de)marshallers.
Apart from this,
Acked-by: Christophe Fergeau
> Avoid spice-gtk and spice-server to use them by mistake.
> This can cause memory errors (data_size is not used or
> is not set correctly) and useless code (spice-gtk uses
> the pub_key* fields but these fields are not sent to
> the server as the protocol does not have them).
>
> Signed-off-by: Frediano Ziglio
> ---
> common/messages.h | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/common/messages.h b/common/messages.h
> index 43d3602..f740a8c 100644
> --- a/common/messages.h
> +++ b/common/messages.h
> @@ -43,7 +43,6 @@
> SPICE_BEGIN_DECLS
>
> typedef struct SpiceMsgData {
> -uint32_t data_size;
> uint8_t data[0];
> } SpiceMsgData;
>
> @@ -75,9 +74,6 @@ typedef struct SpiceMigrationDstInfo {
> uint16_t sport;
> uint32_t host_size;
> uint8_t *host_data;
> -uint16_t pub_key_type;
> -uint32_t pub_key_size;
> -uint8_t *pub_key_data;
> uint32_t cert_subject_size;
> uint8_t *cert_subject_data;
> } SpiceMigrationDstInfo;
> --
> 2.20.1
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-common 2/9] codegen: Factor out a function to write output file
Acked-by: Christophe Fergeau
On Sun, Mar 03, 2019 at 07:10:23PM +, Frediano Ziglio wrote:
> This will be reused to generate C declaration automatically.
>
> Signed-off-by: Frediano Ziglio
> ---
> spice_codegen.py | 46 +-
> 1 file changed, 25 insertions(+), 21 deletions(-)
>
> diff --git a/spice_codegen.py b/spice_codegen.py
> index 76d7c5e..7e22092 100755
> --- a/spice_codegen.py
> +++ b/spice_codegen.py
> @@ -105,6 +105,30 @@ def write_enums(writer, describe=False):
>
> writer.writeln("#endif /* _H_SPICE_ENUMS */")
>
> +def write_content(dest_file, content, keep_identical_file):
> +if keep_identical_file:
> +try:
> +f = open(dest_file, 'rb')
> +old_content = f.read()
> +f.close()
> +
> +if content == old_content:
> +six.print_("No changes to %s" % dest_file)
> +return
> +
> +except IOError:
> +pass
> +
> +f = open(dest_file, 'wb')
> +if six.PY3:
> +f.write(bytes(content, 'UTF-8'))
> +else:
> +f.write(content)
> +f.close()
> +
> +six.print_("Wrote %s" % dest_file)
> +
> +
> parser = OptionParser(usage="usage: %prog [options]
> ")
> parser.add_option("-e", "--generate-enums",
>action="store_true", dest="generate_enums", default=False,
> @@ -290,25 +314,5 @@ if options.header:
> content = writer.header.getvalue()
> else:
> content = writer.getvalue()
> -if options.keep_identical_file:
> -try:
> -f = open(dest_file, 'rb')
> -old_content = f.read()
> -f.close()
> -
> -if content == old_content:
> -six.print_("No changes to %s" % dest_file)
> -sys.exit(0)
> -
> -except IOError:
> -pass
> -
> -f = open(dest_file, 'wb')
> -if six.PY3:
> -f.write(bytes(content, 'UTF-8'))
> -else:
> -f.write(content)
> -f.close()
> -
> -six.print_("Wrote %s" % dest_file)
> +write_content(dest_file, content, options.keep_identical_file)
> sys.exit(0)
> --
> 2.20.1
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v2 1/2] Deprecate “color-depth” properties
Hey, On Wed, Mar 06, 2019 at 02:24:24PM +, Victor Toso wrote: > Hi, > > On Tue, Jan 08, 2019 at 04:22:55PM +0100, Victor Toso wrote: > > Hi, > > > > On Tue, Jan 08, 2019 at 04:09:54PM +0100, Christophe Fergeau wrote: > > > On Fri, Dec 14, 2018 at 04:29:46PM +0100, Victor Toso wrote: > > > > From: Victor Toso > > > > > > > > With commit 1a980f3712 we deprecated some command line options. The > > > > color-depth one is the only one which is not used for testing/debug > > > > purposes. > > > > > > > > The intention of this option is to set guest's video driver color > > > > configuration, currently only windows guest agent uses this feature up > > > > to Windows 7. Windows 8 and up, setting color depth below 32 would > > > > fail. We should not encourage the usage of this feature. > > > > > > > > Related: https://bugzilla.redhat.com/show_bug.cgi?id=1543538 > > > > Related: https://bugzilla.redhat.com/show_bug.cgi?id=1350853 > > > > > > > > This patch deprecates both SpiceMainChannel::color-depth and > > > > SpiceSession::color-depth while ignoring any set/get to it. > > > > > > > > Signed-off-by: Victor Toso > > > > Acked-by: Christophe Fergeau > > > > > > Still fine with me, though it's indeed a good question that Uri raised, > > > whether we want to keep this alive a bit longer for win7 since it's > > > still supported by MS. > > > > | Microsoft ended mainstream support for Windows 7 on January 13, > > | 2015, but extended support won't end until January 14, 2020. > > > > Oh well, IMHO we could remove it for 0.36 but we can do it for > > after 0.36 as well. > > Now that 0.36 is out, I'm wondering if this two patches are fine > for you? CC: teuf and Uri :) Fine with me, Christophe signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
