The items are send from RedChannelClient so move the callback
to a virtual function in RedChannelClient instead of RedChannel.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
The messages coming are from the client so it's a better place
to be in RedChannelClient instead of RedChannel.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Make all RedChannelClient hierarchy a C++ class.
This allows to use virtual methods.
Added a normal contructor instead or properties and g_object_new.
As we remove GObject conversion macros I added a macro XXX_CAST
to create a function to replace the old macro.
They will be removed when more type safety is introduced.
There's a new SPICE_CXX_GLIB_ALLOCATOR macro in red-common.h.
This macro, added to a class define the class allocator allowing
to use, in this case, GLib for allocation. This to avoid C++ library
dependency and to initialize all structure to 0 (not all fields
are manually initialized, will be improved with more encapsulation).
Currently the methods are mainly public, access will be modified
when more encapsulation (all functions in method) are done.
Some classes are now defined in the header, C++ uses access to
limit accessibility but for efficiency and type safety/inline and
other features require types to be defined in the headers.
Some fields were moved from XxxPrivate structure to class, C++
has accessibility.
Many destructors are defined as protected to forbid the use of
stack, this as these objects uses internal reference counting
to have normal pointers. Maybe in the future pointers like
std::shared_ptr could be used instead.
Reference counting is now implemented very easily using atomic
operations.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
If automake sees no C++ files in the source it assumes have to
use C linker settings not linking C++ library.
This was not a problem as code did not use C++ libraries but next
patch will use pure virtual function call.
It could be provided but as later we will use RTTI use C++ library.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Under Darwin SOL_TCP is not defined. Use IPPROTO_TCP instead.
Other part of SPICE server uses this constant instead already.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
attache_worker was always spelled wrongly.
Use anonymous union (standard feature already used in different
code in SPICE) to have an aliased attached_worker function.
Also removed a parameter from this new callback and deprecate
the old.
Due to C ABI removing a parameter is not an issue,
red-qxl.c:red_qxl_attach_worker will continue to pass the parameter,
new code will ignore, old code will receive it.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
It was made obsolete more than 6 years ago by
commit fe0941fb02
Author: Marc-André Lureau <marcandre.lureau@gmail.com>
Date: Thu Oct 3 22:52:38 2013 +0200
server: mark deprecated symbols
For compatibility with spice_replay_next_cmd pass a QXLInstance
pointer. For more information see comment on
red-qxl.c:red_qxl_attach_worker.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
This suppression was present in former glib.supp version however
the suppression in glib.supp was updated to only catch "reachable"
leaks.
In early stages and few objects the leak could be detected as
"possible".
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
This could happen if the machine is pretty busy.
I saw intermittent failures on CI and it happened on my
machine also while doing other batch job.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Better clear the result type instead of a generic "int".
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Kevin Pouget <kpouget@redhat.com>
FALSE is the default value, no need to set explicitly.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Use new common.m4 include file to make easier to integrate
with spice-common repository.
The new include will allow for instance spice-common to
add additional dependencies without changes (or minor) to
spice-server.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Kevin Pouget <kpouget@redhat.com>
g_assert_null, g_assert_true and g_assert_false are all defined in
GLib 2.38
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Basically not run since 03d46e9e "build-sys: Raise glib requirement to
2.38" in 2019-02-05
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Not used since 03d46e9e "build-sys: Raise glib requirement to 2.38" in
2019-02-05
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
The record file uses fprintf to record commands and attributes.
On Windows line terminator uses ASCII format ("\r\n") while on
Linux it used UNIX format ("\n"). Open file as binary to use the
same line terminator on both systems.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Francesco Giudici <fgiudici@redhat.com>
For some reason under Fedora 31 the g_socket_client_connect call
from test-listen test caused Valgrind to detect a leak.
Note that the like at gtype.c (g_socket_client_connect) specifically
marks a memory block as malloc-like. Not sure if it's an issue
with Valgrind or with GLib, as a workaround disable the leak report.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
This suppression was present in former glib.supp version however
the suppression in glib.supp was updated to only catch "reachable"
leaks.
However in test-listen test the allocation is done in a thread
causing the leak to become "definite".
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
Allow to modify/cancel timers/watches without having to retrieve
the code interface.
This will make sure that you are not using the wrong interface.
Simplify code to deal with timers/watches.
Remove the requirement to have the core interface available
for removing timers/watches.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Using coverage utility exercise more code paths:
- message from channel with wrong type;
- remove message from channel with already removed reader;
- init message from channel (ignored);
- data from devices, ADPU;
- error from device;
- messages split in different ways;
- invalid reader_id values.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Create Smardcard device.
Connect to it and test some messages are parsed and processed
as expected.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Allows to reuse code for emulating a character device.
It will be used for Smardcard test.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
As base messages require parsing better channels should always use
the generated parser.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
spice_assert is a macro and covscan reports that:
Argument "++twice_remove_called" of spice_assert() has a side effect.
Doesn't matter if there is a side effects or not,
it's a good practice and it makes covscan happy, so
increment the variable one line above.
Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
There may to be another function (cache_alt_names) between
gnutls_x509_ext_import_subject_alt_names and
gnutls_x509_crt_import
cache_alt_names is a static function in gnutls/lib/x509/x509.c
used only in gnutls_x509_crt_import and may be inlined by
the compiler.
Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Threads are joined by spice_server_destroy, just need to release
last resources.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Snir Sheriber <ssheribe@redhat.com>
Previously we add suppression to glib.supp file (suppressions from
Glib).
Keep the glib.supp file pristine and add another file specific
for SPICE.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
The 0 result means success however the function (correctly) could
report a failure if the string is incorrect.
This fixes the test after commit b4150de3cd
("spice_server_set_video_codecs: fail when no codec can be installed").
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Snir Sheriber <ssheribe@redhat.com
The WebSocket protocol allows 0-size frames so a returned lenth of
0 does not only mean an issue but it's perfectly expected.
This is also required by WebSocket specification.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jeremy White <jwhite@codeweavers.com>
Allows to specify and get frame type.
Type and flags are returned calling websocket_read and returned
calling websocket_write or websocket_writev.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jeremy White <jwhite@codeweavers.com>
The G_PID_FORMAT constant is defined only if GLib does not support it.
The constant was wrongly defined.
Jessie Debian 32 shows this issue (printf format error).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jeremy White <jwhite@codeweavers.com>
On ppc64el and armhf the handling of "1 << mem_info.memslot_id_shift"
will end up beign a zero which breaks the test.
Marking the implicit value 1 as a 64 bit value (to match the uint64_t
target) fixes the issue.
Fixes#31
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This change prevent a warning issued by GCC 9 and potentially
other compilers.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
Although this feature can be ported to Windows doing so would
require the usage of g_spawn_async_with_fds, which is only available
in GLib 2.58 or some specific Win32 code.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
test-stream test is passing file descriptor using Unix socket.
test-stat-file needs some porting work of mmap feature.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
This should always be defined and including config.h is a requirement.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Rather than having an API to register client callbacks for each channel
type, make them vfuncs.
Since the client callbacks are registered identically for each channel
of the same type, it doesn't make sense for to require these to be
registered separately for each object. It's cleaner to have these be
per-class properties, so they've been converted to virtual functions.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
We should not reuse the same interface twice as doing so will
cause dangling pointers.
Unregister it at every iteration.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
usleep under Windows does not seem to have the required precision.
Use milliseconds and adjust check times according.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
RedMemSlotInfo keeps an array of groups, and each group contains an
array of slots. Unfortunately, these checks are off by 1, they check
that the index is greater or equal to the number of elements in the
array, while these arrays are 0 based. The check should only check for
strictly greater than the number of elements.
For the group array, this is not a big issue, as these memslot groups
are created by spice-server users (eg QEMU), and the group ids used to
index that array are also generated by the spice-server user, so it
should not be possible for the guest to set them to arbitrary values.
The slot id is more problematic, as it's calculated from a QXLPHYSICAL
address, and such addresses are usually set by the guest QXL driver, so
the guest can set these to arbitrary values, including malicious values,
which are probably easy to build from the guest PCI configuration.
This patch fixes the arrays bound check, and adds a test case for this.
This fixes CVE-2019-3813.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Client process termination did not work for Windows, used Win32
APIs.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Windows does not support Unix sockets.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
This is a preparatory patch for next portability patches
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Due to previous commit "make syntax-check" command reports:
prohibit_signal_without_use
server/tests/test-display-base.c
maint.mk: the above files include signal.h but don't use it
make: *** [maint.mk:639: sc_prohibit_signal_without_use] Error 1
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Use GLib function to launch and wait process exit.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
If input contains the binary record we can't have it modified
during read.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Split level computation, make clear is a sine wave on both channels.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
We are waiting for a client connection, channel is already there
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
This changes tests/pki/server-cert.pem and tests/pki/ca-cert.pem to have
2048 bits. These certificates were generated using the
instructions on https://www.spice-space.org/spice-user-manual.html
The -subj args were omitted, and the defaults suggested by openssl used.
The -days parameter was changed to -days 10950, the bits to 2048.
This fixes https://gitlab.freedesktop.org/spice/spice/issues/27.
Some distros are starting to use stricter settings for their openssl
configuration, which forbids 1024 bit keys, and causes test suite
failures.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Currently, RedWorker is using stack-allocated variables for RedSurfaceCmd.
Surface commands are rare enough that we can dynamically allocate them
instead, and make the API in red-parse-qxl.h consistent with how other
QXL commands are handled.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Currently, the cursor channel is allocating RedCursorCmd instances itself, and then
calling into red-parse-qxl.h to initialize it, and doing something
similar when releasing the data. This commit moves this common code to
red-parse-qxl.[ch]
The ref/unref are not strictly needed, red_cursor_cmd_free() would
currently be enough, but this makes the API consistent with
red_drawable_{new,ref,unref}.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Just a style change, on more recent GLib would print a more
friendly error report.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Is possible that port 5913 is already in use as tests that uses
test_new will attempt to use ports from 5912 to 5921 so use a port
not in that range.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
test_new function attempts to detect attempts to listen to tcp ports
already in listening state detecting some messages during
spice_server_init. However the check is wrong (broken in recent
34a44d3e94 "test-display-base: Avoid spurious errors due to listen
failures") and incomplete (missing message).
To better test this conditions put some of the ports in listening
state (like with a "nc -l 5912 & nc -l 5913 &" command) and run
tests in parallel (like with a "make check -j" command).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
To set up a listening socket usually you call in sequence:
- socket;
- bind;
- listen.
If you try to bind() to a port when another socket is already
listening on that port, the bind() will fail.
However, it is possible that the bind() may succeed and the listen()
will fail, as demonstrated in the following sequence:
- socket() create socket 1;
- bind() to port N on socket 1;
- socket() create socket 2;
- bind() to port N on socket 2;
- listen() on socket 1;
- listen() on socket 2 <-- failure.
When running tests (especially multiple tests running in parallel), it
may sometimes happen that there are other tests already listening on
the port that we are trying to use. In this case, we want to ignore
this error and simply try to listen on a different port. We already
attempted to handle this scenario, but we were only ignoring bind()
errors and not listen() errors. So in the scenario mentioned above,
the listen() error was causing the entire test to fail instead of
allowing us to try to listen on another port.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
In a comparison with current autotools build system, meson/ninja
provides a huge improvement in build speed, while keeping the same
functionalities currently available and being considered more user
friendly.
The new system coexists within the same repository with the current one,
so we can do more extensive testing of its functionality before deciding
if the old system can be removed, or for some reason, has to stay for
good.
- Meson: https://mesonbuild.com
This is the equivalent of autogen/configure step in autotools. It
generates the files that will be used by ninja to actually build the
source code.
The project has received lots of traction recently, with many GNOME
projects willing to move to this new build system. The following wiki
page has more details of the status of the many projects being ported:
https://wiki.gnome.org/Initiatives/GnomeGoals/MesonPorting
Meson has a python-like syntax, easy to read, and the documentation
on the project is very complete, with a dedicated page on how to port
from autotools, explaining how most common use cases can be
implemented using meson.
http://mesonbuild.com/Porting-from-autotools.html
Other important sources of information:
http://mesonbuild.com/howtox.htmlhttp://mesonbuild.com/Syntax.htmlhttp://mesonbuild.com/Reference-manual.html
- Ninja: https://ninja-build.org
Ninja is the equivalent of make in an autotools setup, which actually
builds the source code. It has being used by large and complex
projects such as Google Chrome, Android and LLVM. There is not much to
say about ninja (other than it is much faster than make) because we
won't interact directly with it as much, as meson does the middle man
job here. The reasoning for creating ninja in the first place is
explained on the following post:
http://neugierig.org/software/chromium/notes/2011/02/ninja.html
Also its manual provides more in-depth information about the design
principles:
https://ninja-build.org/manual.html
- Basic workflow:
Meson package is available for most if not all distros, so, taking
Fedora as an example, we only need to run:
# dnf -y install meson ninja-build.
With Meson, building in-tree is not possible at all, so we need to
pass a directory as argument to meson where we want the build to be
done. This has the advantage of creating builds with different options
under the same parent directory, e.g.:
$ meson ./build --prefix=/usr
$ meson ./build-extra -Dextra-checks=true -Dalignment-checks=true
After configuration is done, we call ninja to actually do the build.
$ ninja -C ./build
$ ninja -C ./build install
Ninja defaults to parallel builds, and this can be changed with the -j
flag.
$ ninja -j 10 -C ./build
- Hacking:
* meson.build: Mandatory for the project root and usually found under
each directory you want something to be built.
* meson_options.txt: Options that can interfere with the result of the
build.
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Fixes test-stream-device after adding a log warning about an invalid
message received on the stream device, glib tests fail on unexpected
warning messages.
Signed-off-by: Lukáš Hrázký <lhrazky@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
The reason for this commit is that Meson expects all submodules to be
placed in this subdirectory, and since autotools build is more flexible
in this case, we make some small adjustments to configure.ac and
Makefile.am files to accommodate for this change.
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Check that data sent to device are collapsed in a single message.
The StreamChannel object is mocked in the test.
This checks that commit dcc3f995d9
("stream-device: handle_data: send whole message") is doing the
right thing.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
If guest sent an empty data message this was not parsed correctly.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
G_PID_FORMAT was only added in glib 2.50.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
In case we pass something like "spice:mjpeg$%*" the last part is
ignore making the string parse correctly.
A single pair should end by either string terminator or pair terminator.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
