[Spice-devel] [PATCH spice-gtk] egl: Adjust to window scaling
When GDK_SCALE is != 1 and egl is used, the presented image does not fit to the window (scale of 2 is often used with hidpi monitors). Usually this is not a problem since all components are adjusted by gdk/gtk but with egl, pixel-based data is not being scaled. In this case window's scale value can be used in order to determine whether to use a pixel resource with higher resolution data. --- In order to reproduce the problem set spice with virgl/Intel-vGPU and run spice-gtk with GDK_SCALE=2 This patch is kind of RFC, it fixes the issue, but it's a bit hacky and specific. I didn't come across other scale issues but it is likely that more of these exist and better and generic fix is needed. --- src/spice-widget-egl.c | 15 +-- src/spice-widget-priv.h | 1 + 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/spice-widget-egl.c b/src/spice-widget-egl.c index 43fccd7..814a580 100644 --- a/src/spice-widget-egl.c +++ b/src/spice-widget-egl.c @@ -326,6 +326,8 @@ static gboolean spice_widget_init_egl_win(SpiceDisplay *display, GdkWindow *win, if (d->egl.surface) return TRUE; +d->egl.scale = gdk_window_get_scale_factor(win); + #ifdef GDK_WINDOWING_X11 if (GDK_IS_X11_WINDOW(win)) { native = (EGLNativeWindowType)GDK_WINDOW_XID(win); @@ -431,15 +433,17 @@ void spice_egl_resize_display(SpiceDisplay *display, int w, int h) { SpiceDisplayPrivate *d = display->priv; int prog; +gint ws; if (!gl_make_current(display, NULL)) return; +ws = d->egl.scale; glGetIntegerv(GL_CURRENT_PROGRAM, &prog); glUseProgram(d->egl.prog); -apply_ortho(d->egl.mproj, 0, w, 0, h, -1, 1); -glViewport(0, 0, w, h); +apply_ortho(d->egl.mproj, 0, w * ws , 0, h * ws, -1, 1); +glViewport(0, 0, w * ws, h * ws); if (d->ready) spice_egl_update_display(display); @@ -559,6 +563,13 @@ void spice_egl_update_display(SpiceDisplay *display) spice_display_get_scaling(display, &s, &x, &y, &w, &h); +// Adjust to gdk scale +s *= d->egl.scale; +x *= d->egl.scale; +y *= d->egl.scale; +w *= d->egl.scale; +h *= d->egl.scale; + glClearColor(0.0f, 0.0f, 0.0f, 0.0f); glClear(GL_COLOR_BUFFER_BIT); diff --git a/src/spice-widget-priv.h b/src/spice-widget-priv.h index 65eb404..8f110ac 100644 --- a/src/spice-widget-priv.h +++ b/src/spice-widget-priv.h @@ -149,6 +149,7 @@ struct _SpiceDisplayPrivate { EGLImageKHR image; gbooleancall_draw_done; SpiceGlScanout scanout; +gintscale; } egl; #endif // HAVE_EGL double scroll_delta_y; -- 2.19.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [vdagent-win PATCH] file-xfer: handle_start: use snprintf instead of sprintf_s
On 2/28/19 3:12 PM, Frediano Ziglio wrote: On 2/25/19 4:12 PM, Frediano Ziglio wrote: When building with older mingw, sprintf_s does not always work as expected, but snprintf does. Also it's more consistent in the file. Note that when building with VS, snprintf becomes sprintf_s I think this could be a bug, from documentation: "If the buffer is too small for the formatted text, including the terminating null, then the buffer is set to an empty string by placing a null character at buffer[0], and the invalid parameter handler is invoked" which is different from snprintf behaviour, _snprintf is probably what do we want. Actually, I think we do want snprintf's behavior, not _snprintf. _snprintf does not guarantee null termination of the string. Yes, you are right, sorry for the confusion. We should probably check the return value. Well, yes, and in call to snprintf, even on Linux. Or continue to ignore as we do. Also we can add a check that there is enough space and fail early. This does not make sense, better to check later, it's easier. But usually you just call snprintf and ignore if truncated, I don't see why in this case we should make an exception. There is a check already with +3. We can make it with +1+6, so we know there is enough space: diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp index c456bbe..7249b21 100644 --- a/vdagent/file_xfer.cpp +++ b/vdagent/file_xfer.cpp @@ -93,8 +93,9 @@ void FileXfer::handle_start(VDAgentFileXferStartMessage* start, wlen = _tcslen(file_path); // make sure we have enough space -// (1 char for separator, 1 char for filename and 1 char for NUL terminator) -if (wlen + 3 >= MAX_PATH) { +// 1 = char for directory separator: "\" +const size_t POSTFIX_LEN = 6; // up to 2 digits in parentheses and final NUL: " (xx)" +if (wlen + 1 + POSTFIX_LEN >= MAX_PATH) { vd_printf("error: file too long %ls\\%s", file_path, file_name); return; } Uri. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [vdagent-win PATCH] file-xfer: handle_start: use snprintf instead of sprintf_s
> On 2/28/19 3:12 PM, Frediano Ziglio wrote: > >> On 2/25/19 4:12 PM, Frediano Ziglio wrote: > > When building with older mingw, sprintf_s does not > always work as expected, but snprintf does. > > Also it's more consistent in the file. > > Note that when building with VS, snprintf becomes sprintf_s > > >>> > >>> I think this could be a bug, from documentation: > >>> > >>> "If the buffer is too small for the formatted text, including the > >>> terminating null, then the buffer is set to an empty string by placing a > >>> null character at buffer[0], and the invalid parameter handler is > >>> invoked" > >>> > >>> which is different from snprintf behaviour, _snprintf is probably what do > >>> we want. > >> > >> Actually, I think we do want snprintf's behavior, not _snprintf. > >> _snprintf does not guarantee null termination of the string. > >> > > > > Yes, you are right, sorry for the confusion. > > > >> We should probably check the return value. > > > > Well, yes, and in call to snprintf, even on Linux. Or continue to > > ignore as we do. > > > >> Also we can add a check that there is enough space and fail early. > >> > > > > This does not make sense, better to check later, it's easier. > > But usually you just call snprintf and ignore if truncated, > > I don't see why in this case we should make an exception. > > > There is a check already with +3. That check is protecting file_path, not dest_filename. > We can make it with +1+6, so we know there is enough space: > It's not enough, but the overflow on file_path is already checked checking the return value of MultiByteToWideChar > diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp > index c456bbe..7249b21 100644 > --- a/vdagent/file_xfer.cpp > +++ b/vdagent/file_xfer.cpp > @@ -93,8 +93,9 @@ void > FileXfer::handle_start(VDAgentFileXferStartMessage* start, > > wlen = _tcslen(file_path); > // make sure we have enough space > -// (1 char for separator, 1 char for filename and 1 char for NUL > terminator) > -if (wlen + 3 >= MAX_PATH) { > +// 1 = char for directory separator: "\" > +const size_t POSTFIX_LEN = 6; // up to 2 digits in parentheses and > final NUL: " (xx)" > +if (wlen + 1 + POSTFIX_LEN >= MAX_PATH) { > vd_printf("error: file too long %ls\\%s", file_path, file_name); > return; > } > > Uri. > Looking at the code char dest_filename[SPICE_N_ELEMENTS(file_name) + POSTFIX_LEN]; if (attempt == 0) { strcpy(dest_filename, file_name); } else { sprintf_s(dest_filename, SPICE_N_ELEMENTS(dest_filename), "%.*s (%d)%s", int(extension - file_name), file_name, attempt, extension); } so the dest_filename is large enough, you could even replace sprintf_s with a sprintf. Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-server v4 00/20] Port SPICE server to Windows
ping > > ping > > > > > Windows support is useful to use with Qemu under Windows as host or > > to implement servers like Xspice. > > Mainly SPICE server uses lot of libraries to expose a TCP protocol. > > As TCP is implemented with socket library which is quite portable was > > not that hard to port. > > Beside some minor feature (see REAME.Windows) all was ported. > > During porting was choosen to keep Unix as the main platform, if a > > change would require too much changes some Windows wrapper is > > preferred instead. Not too complicated stuff, the main "wrapper" is > > that Windows errors from socket are written back into errno to avoid > > to change lot of code handling errors. > > > > Changes since v3: > > - reduce compatibility layer size: > > - do not change function name if not required; > > - use int instead of a new socket_t. > > > > Changes since v2: > > - better %m replacement; > > - many comments updates; > > - typo and grammar fixes; > > - use int to store socket descriptors/handles; > > - merge all v2 updates into a single series; > > - split formatting string patch. > > > > Frediano Ziglio (20): > > Use proper format strings for spice_log > > build: Detect Windows build and change some definitions > > Avoids %m in formatting for Windows > > windows: Undefine some conflicting preprocessor macros > > tests: Provide alarm replacement for Windows > > test-stat: Adjust delay checks > > sys-socket: Introduce some utility to make sockets more portable > > sys-socket: Add socket_newpair utility > > net-utils: Use socket compatibility layer > > reds: Use socket compatibility layer > > red-stream: Use socket compatibility layer > > dispatcher: Use socket compatibility layer > > test-leaks: Use socket compatibility layer > > test-channel: Use socket compatibility layer > > windows: Disable code not working on Windows > > dispatcher: Port to Windows > > tests: Exclude tests that cannot work on Windows > > red-stream: Fix SSL connection for Windows > > Disable recording filtering for Windows > > Add some notes for the Windows port > > > > README.Windows| 18 ++ > > configure.ac | 20 ++- > > server/Makefile.am| 2 + > > server/char-device.c | 3 +- > > server/dispatcher.c | 28 ++- > > server/gstreamer-encoder.c| 4 +- > > server/main-channel-client.c | 6 +- > > server/memslot.c | 2 +- > > server/mjpeg-encoder.c| 6 +- > > server/net-utils.c| 11 ++ > > server/red-channel-client.c | 2 + > > server/red-channel.c | 6 +- > > server/red-client.c | 6 +- > > server/red-common.h | 1 + > > server/red-qxl.c | 4 + > > server/red-record-qxl.c | 7 + > > server/red-replay-qxl.c | 9 +- > > server/red-stream.c | 48 - > > server/red-stream.h | 2 + > > server/red-worker.c | 6 + > > server/reds.c | 53 +++--- > > server/sound.c| 5 +- > > server/stat-file.c| 2 + > > server/sys-socket.c | 287 ++ > > server/sys-socket.h | 142 +++ > > server/tests/Makefile.am | 11 +- > > server/tests/basic-event-loop.c | 2 + > > server/tests/replay.c | 2 + > > server/tests/stat-test.c | 12 +- > > server/tests/test-channel.c | 9 +- > > server/tests/test-leaks.c | 5 +- > > server/tests/test-loop.c | 1 + > > server/tests/test-record.c| 7 +- > > server/tests/test-stream-device.c | 1 + > > server/tests/win-alarm.c | 65 +++ > > server/tests/win-alarm.h | 26 +++ > > tools/Makefile.am | 2 + > > 37 files changed, 753 insertions(+), 70 deletions(-) > > create mode 100644 README.Windows > > create mode 100644 server/sys-socket.c > > create mode 100644 server/sys-socket.h > > create mode 100644 server/tests/win-alarm.c > > create mode 100644 server/tests/win-alarm.h > > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-common 1/9] messages: Remove fields not used by the protocol
These fields are not used by the protocol. 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
[Spice-devel] [PATCH spice-common 3/9] codegen: Generate headers while generating code
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" 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', 'common/messages.h', '@INPUT@', '@OUTPUT@']], -['server_marshallers', spice_proto, 'generated_server_marshallers.c', ['--generate-marshallers', '--server'] + structs_args + ['--include', 'common/messages.h', '@INPUT@', '@OUTPUT@']], -['server_marshallers_h', spice_proto, 'generated_server_marshallers.h', ['--g
[Spice-devel] [PATCH spice-common 8/9] protocol: Add a dummy TunnelChannel
The removal of the channel definition will cause the enumeration to miss this old channel. Add a dummy channel (empty) to avoid having to update spice-gtk and spice-server and possibly breaking other software. Signed-off-by: Frediano Ziglio --- spice.proto | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/spice.proto b/spice.proto index de80b27..1f9f57b 100644 --- a/spice.proto +++ b/spice.proto @@ -1235,6 +1235,9 @@ channel RecordChannel : BaseChannel { } @declare start_mark; }; +channel TunnelChannel { +}; + enum32 vsc_message_type { Init = 1, Error, @@ -1357,8 +1360,8 @@ protocol Spice { CursorChannel cursor; PlaybackChannel playback; RecordChannel record; -// there used to be a TunnelChannel -SmartcardChannel smartcard = 8; +TunnelChannel tunnel; +SmartcardChannel smartcard; UsbredirChannel usbredir; PortChannel port; WebDAVChannel webdav; -- 2.20.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-common 4/9] codegen: Allows to generate C declarations automatically
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)) +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, members, attribute_list): Type.__init__(self) diff --git a/spice_codegen.py b/spice_codegen.py index 4664740..66f99a5 100755 --- a/spice_codegen.py +++ b/spice_codegen.py @@ -177,6 +177,8 @@ parser.add_option("--license", dest="license", help="license to use for generated file(s) (LGPL/BSD)", default="LGPL") parser.add_option("--generated-header", dest="generated_header", metavar="FILE", help="Name of the file to generate the header") +parse
[Spice-devel] [PATCH spice-common 2/9] codegen: Factor out a function to write output file
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
[Spice-devel] [PATCH spice-common 7/9] codegen: Remove support for --ptrsize
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(&m2->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 set_ptr_size(self, new_size): -self.pointer_size = new_size - def is_fixed_nw_size(self): return True @@ -531,19 +525,13 @@ class PointerType(Type): return True def primitive_type(self): -if self.pointer_size == 4: -return "uint32" -else: -return "uint64" +return
[Spice-devel] [PATCH spice-common 0/9] Miscellaneous code generation improvements
Part of the series already sent. Frediano Ziglio (9): messages: Remove fields not used by the protocol codegen: Factor out a function to write output file codegen: Generate headers while generating code codegen: Allows to generate C declarations automatically Allow to generate C declarations for spice.proto Generate automatically most C message declarations codegen: Remove support for --ptrsize protocol: Add a dummy TunnelChannel codegen: Rename --prefix parameter to --suffix common/Makefile.am | 22 +- common/client_marshallers.h | 2 +- common/marshaller.c | 14 +- common/marshaller.h | 2 +- common/meson.build | 29 ++- common/messages.h | 497 +--- python_modules/codegen.py | 2 +- python_modules/demarshal.py | 10 +- python_modules/marshal.py | 6 +- python_modules/ptypes.py| 82 -- spice.proto | 199 --- spice_codegen.py| 115 ++--- 12 files changed, 307 insertions(+), 673 deletions(-) -- 2.20.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-common 5/9] Allow to generate C declarations for spice.proto
Generate and include C declarations. Next patch will use this facility. 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\ $(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
[Spice-devel] [PATCH spice-common 9/9] codegen: Rename --prefix parameter to --suffix
The option is used to add a suffix to public functions, not a prefix. Currently the option is not used (it was used to generate protocol 1 code). Signed-off-by: Frediano Ziglio --- python_modules/codegen.py | 2 +- python_modules/demarshal.py | 10 +- python_modules/marshal.py | 4 ++-- spice_codegen.py| 8 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/python_modules/codegen.py b/python_modules/codegen.py index f7a2048..bfb2351 100644 --- a/python_modules/codegen.py +++ b/python_modules/codegen.py @@ -117,7 +117,7 @@ class CodeWriter: writer.index_type = self.index_type writer.generated = self.generated writer.options = self.options -writer.public_prefix = self.public_prefix +writer.public_suffix = self.public_suffix return writer diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py index e59521c..d3147b7 100644 --- a/python_modules/demarshal.py +++ b/python_modules/demarshal.py @@ -1136,9 +1136,9 @@ def write_channel_parser(writer, channel, server): def write_get_channel_parser(writer, channel_parsers, max_channel, is_server): writer.newline() if is_server: -function_name = "spice_get_server_channel_parser" + writer.public_prefix +function_name = "spice_get_server_channel_parser" + writer.public_suffix else: -function_name = "spice_get_client_channel_parser" + writer.public_prefix +function_name = "spice_get_client_channel_parser" + writer.public_suffix scope = writer.function(function_name, "spice_parse_channel_func_t", @@ -1195,15 +1195,15 @@ def write_full_protocol_parser(writer, is_server): function_name = "spice_parse_msg" else: function_name = "spice_parse_reply" -scope = writer.function(function_name + writer.public_prefix, +scope = writer.function(function_name + writer.public_suffix, "uint8_t *", "uint8_t *message_start, uint8_t *message_end, uint32_t channel, uint16_t message_type, SPICE_GNUC_UNUSED int minor, size_t *size_out, message_destructor_t *free_message") scope.variable_def("spice_parse_channel_func_t", "func" ) if is_server: -writer.assign("func", "spice_get_server_channel_parser%s(channel, NULL)" % writer.public_prefix) +writer.assign("func", "spice_get_server_channel_parser%s(channel, NULL)" % writer.public_suffix) else: -writer.assign("func", "spice_get_client_channel_parser%s(channel, NULL)" % writer.public_prefix) +writer.assign("func", "spice_get_client_channel_parser%s(channel, NULL)" % writer.public_suffix) with writer.if_block("func != NULL"): writer.statement("return func(message_start, message_end, message_type, minor, size_out, free_message)") diff --git a/python_modules/marshal.py b/python_modules/marshal.py index 74f3a54..a09b614 100644 --- a/python_modules/marshal.py +++ b/python_modules/marshal.py @@ -412,10 +412,10 @@ def write_protocol_marshaller(writer, proto, is_server, private_marshallers): writer.header.end_block(newline=False) writer.header.writeln(" SpiceMessageMarshallers;") writer.header.newline() -writer.header.statement("SpiceMessageMarshallers *spice_message_marshallers_get" + writer.public_prefix+"(void)") +writer.header.statement("SpiceMessageMarshallers *spice_message_marshallers_get" + writer.public_suffix+"(void)") writer.header.newline() -scope = writer.function("spice_message_marshallers_get" + writer.public_prefix, +scope = writer.function("spice_message_marshallers_get" + writer.public_suffix, "SpiceMessageMarshallers *", "void") writer.writeln("static SpiceMessageMarshallers marshallers = {NULL};").newline() diff --git a/spice_codegen.py b/spice_codegen.py index e518686..4f096a2 100755 --- a/spice_codegen.py +++ b/spice_codegen.py @@ -169,8 +169,8 @@ parser.add_option("-k", "--keep-identical-file", parser.add_option("-i", "--include", action="append", dest="includes", metavar="FILE", help="Include FILE in generated code") -parser.add_option("--prefix", dest="prefix", - help="set public symbol prefix", default="") +parser.add_option("--suffix", dest="suffix", + help="set public symbol suffix", default="") parser.add_option("--license", dest="license", help="license to use for generated file(s) (LGPL/BSD)", default="LGPL") parser.add_option("--generated-header", dest="generated_header", metavar="FILE", @@ -284,7 +284,7 @@ current: def generate_declarations(): writer = codegen.CodeWriter() -writer.public_prefix = options.prefix +writer.public_suffix = options.suffix writer.write(license) # all types @@ -3
[Spice-devel] [PATCH spice-common 6/9] Generate automatically most C message declarations
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; -} SpiceMsgMainAgentTokens, SpiceMsgcMainAgentTokens, SpiceMsgcMainAgentStart; - -typedef struct SpiceMsgMainAgentTokens SpiceMsgMainAgentConnectedTokens; - -typedef struct SpiceMsgcClientInfo { -uint64_t cache_size; -} SpiceMsgcClientInfo; - -typedef struct SpiceMsgcMainMouseModeRequest { -uint32_t mode; -} SpiceMsgcMainMouseModeRequest; - -typedef struct SpiceCursor { -uint32_t flags; -SpiceCursorHeader header; -uint32_t data_size; -uint8_t *data; -} SpiceCursor; - -typedef struct SpiceMsgDisplayMode { -uint32_t x_res; -uint32_t y_res; -uint32_t bits; -} SpiceMsgDisplayMode; - -typedef struct SpiceMsgSurfaceCreate { -uint32_t surface_id; -uint32_t width; -uint32_t height; -uint32_t format; -uint32_t flags; -} SpiceMsgSurfaceCreate; - -typedef struct SpiceMsgS
Re: [Spice-devel] [vdagent-win PATCH] file-xfer: handle_start: use snprintf instead of sprintf_s
On 3/3/19 12:26 PM, Uri Lublin wrote: On 2/28/19 3:12 PM, Frediano Ziglio wrote: On 2/25/19 4:12 PM, Frediano Ziglio wrote: When building with older mingw, sprintf_s does not always work as expected, but snprintf does. Also it's more consistent in the file. Note that when building with VS, snprintf becomes sprintf_s I think this could be a bug, from documentation: "If the buffer is too small for the formatted text, including the terminating null, then the buffer is set to an empty string by placing a null character at buffer[0], and the invalid parameter handler is invoked" which is different from snprintf behaviour, _snprintf is probably what do we want. Actually, I think we do want snprintf's behavior, not _snprintf. _snprintf does not guarantee null termination of the string. Yes, you are right, sorry for the confusion. We should probably check the return value. Well, yes, and in call to snprintf, even on Linux. Or continue to ignore as we do. Also we can add a check that there is enough space and fail early. This does not make sense, better to check later, it's easier. But usually you just call snprintf and ignore if truncated, I don't see why in this case we should make an exception. There is a check already with +3. We can make it with +1+6, so we know there is enough space: On second thought, that's not good. 1. The filename length is not in the calculation 2. It's possible that the file is long and can be created, but with the (xx) addition it's too long. Uri ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel