The shift of a uint_8 number by a number > 32 causes an overflow.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Uri Lublin <ulublin@redhat.com>
This regression was caused by commit
"2ffa7d00c60808e2f640df9bc9b5d62598455588"
(cfr "Improve encapsulation for InputsChannelClient")
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Rope <jrope@redhat.com>
The spice-wrapped.h was introduced to deal with such tricks
in the code and limit them to a single place.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Rope <jrope@redhat.com>
Compiling under ARM7 GCC compiler report these warning:
image-encoders.cpp: In function 'int lz4_usr_more_space(Lz4EncoderUsrContext*, uint8_t**)':
image-encoders.cpp:261:32: error: cast from 'Lz4EncoderUsrContext*' to 'Lz4Data*' increases required alignment of target type [-Werror=cast-align]
261 | EncoderData *usr_data = &(((Lz4Data *)usr)->data);
| ^~~~~~~~~~~~~~
image-encoders.cpp: In function 'int lz4_usr_more_lines(Lz4EncoderUsrContext*, uint8_t**)':
image-encoders.cpp:329:32: error: cast from 'Lz4EncoderUsrContext*' to 'Lz4Data*' increases required alignment of target type [-Werror=cast-align]
329 | EncoderData *usr_data = &(((Lz4Data *)usr)->data);
| ^~~~~~~~~~~~~~
In all other occurrences of these cast we are already using
SPICE_CONTAINEROF.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Rope <jrope@redhat.com>
Using socket pairs and trying to set a TCP level option on the
socket setsockopt returns EINVAL error and not ENOTSUP as
expected.
Check this case and handle as ENOTSUP (ignoring it).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Darwin uses also the constant EOPNOTSUPP for the same reasons.
In some versions EOPNOTSUPP is defined as ENOTSUP.
Check both values, not only ENOTSUP.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Darwin uses for the same setting the TCP_KEEPALIVE option.
Use TCP_KEEPALIVE instead of TCP_KEEPIDLE for Darwin.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Darwin does not have MSG_NOSIGNAL but allows to set a SO_NOSIGPIPE
option to disable sending SIGPIPE writing to closed socket.
Note that *BSD has the SO_NOSIGPIPE option but does not affect all
write calls so instead continue to use MSG_NOSIGNAL instead on
that systems.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
When `build-aux/git-version-gen` is not able to find the project
version (no git tags and no `.tarball-version`), it returns 'UNKNOWN'
(instead of the project version like `0.14.3.185-15b1`).
This `UNKNOWN` value fails `meson build` command:
server/meson.build:8:0: ERROR: Index 1 out of bounds of array of size 1.
With this patch, we set a default major/minor/micro value when the
actual version is unknown. This is the same as the `autoconf`
historical behavior.
Fixes#41.
Signed-off-by: Kevin Pouget <kpouget@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
When SASL is active, if a read request is made and SASL buffer contains some
data (but not enough to fulfill the request), upon return the taken data from
the buffer is not accounted for and hence part of the message gets discarded.
red_stream_sasl_read function takes available data from sasl buffer and returns
if it's enough. If it's not, nbyte is decremented and buf pointer is
incremented to account for the taken data (if any). Then it tries to get more
data from the socket and decode it.
Suppose there was some data in the sasl buffer, but not enough. Then the socket
is not readable (EAGAIN, EINTR, whatever) or the new data isn't enough for
sasl_decode (hence decodedlen == 0). In both cases the function returns as if
no data was read, but it took some data from sasl buffer. This data is lost and
from this point on the communication ceases on the channel (eventually new data
is read, but messages are corrupt without the parts previously discarded).
On the other hand, if some data is read from sasl buffer and everything else
works fine, the output buffer contains all the data, but the count returned
only inform the caller about the newly read data (which causes the similar
effect of discarding part of the message).
Fixes: https://gitlab.freedesktop.org/spice/spice/-/issues/40
Acked-by: Frediano Ziglio <fziglio@redhat.com>
We don't use anymore GObject parameters so avoid having to
register enum values to GType system to use them.
We just need to get the nick value of the enum values for
debug purposes.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Instead of including spice.h directly include an header that wraps
it. This allows to remove the SPICE_SERVER_INTERNAL define.
Currently is used to rename SpiceCharDeviceInstance to RedCharDevice
and reduce its visibility to hidden. This remove some warnings
and some weird code in the source.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
This allows to make easier the management of owning.
Reference counting is automatically updated based on shared
pointers modification.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
The reason to not using STL is that our code from how was designed requires
the iterator to be safe to the delete of the element pointed by the iterator.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
RedCharDevice can all be removed just calling unref, beside
the agent that needs special threatment.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Use a template to wrap the other dispatcher_send_message_custom
avoiding having to pass a void* opaque and extract payload size
from passed type.
Will be used more by next commit when Dispatchers are turned into
C++.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
The only notify was done from the same file.
All other read of the property were replaced.
Preparing to remove GObject.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Add MainChannelClient::get_channel to avoid to manually cast
to derived type.
Pass objects as MainChannelClient instead of RedChannelClient if
we need a MainChannelClient.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
The patch seems pretty huge but mainly are mechanical steps:
- remove GObject declarations
- do not inherit from GObject
- add SPICE_CXX_GLIB_ALLOCATOR to avoid using C++ allocators
- CLASS_init and CLASS_constructor code goes into C++ constructor
- CLASS_dispose and CLASS_finalize code goes into C++ destructor
- g_object_new is replaced by new operator
- class members goes into virtual methods
- class parameters became argument to constructor
- use push-visibility.h and pop-visibility.h to limit visibility
- temporary use XXX_CAST for old GObject casts, they will
be replaced
- g_object_get is replaced by accessors
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
This allows the compiler to do some more optimisations on the
produced binary.
To allows possible future portability include header/footer in
some helper header files.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Instead of forcibly cast functions cast only if data pointer and
function pointers match. This also allows to remove dangerous
casts all over the place.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Virtual functions cannot be null so use a return value instead
and return the serial using a reference.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Move most inputs_channel_client_* functions inside the class.
This also helps preparing handle_migrate_data to be virtual.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
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>
Initialise RedChannelClientPrivate fields from the new
constructor instead from RedChannelClient.
Also change some fields to constants (actually many of them).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Improve incapsulation.
The only not mechanical change is the addition of timer_add to
make timer settings a bit more type safe.
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>
C++ thread enum a bit more safely than C not allowing to combine
them. Avoid warning combining them and passing to functions
expecting enums.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Some Windows socket functions accepts char* instead of classic void*
causing some warning. Force the cast.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Empty structure are not really portable however adding
an zero size array seems to be the way to have a zero
size structure portably.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Remove -Werror and add -fpermissive, this will allow to compile C code with
a GNU C++ compiler.
Ignore warnings as our code use some feature like empty arrays.
Remove warnings not available in C++.
Bump GLIB_VERSION_MAX_ALLOWED to reduce the warning, looks like the
GLib headers for C++ are not able to handle them correctly.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Memory pool handling does not give much but make code more complex.
Current implementation tends to only increasing buffer sizes
leading to potential cases where most of the allocated memory is
not used.
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>
Inline the function directly, avoid useless function and developers
having to go up and down the code just to understand what these
function do.
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>
The existing .pc file had the wrong dependencies for Windows and was
missing the optional private dependencies for static linking.
Signed-off-by: James Le Cuirot <chewi@gentoo.org>
Acked-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>
RedVmcChannel can have only one client so check directly it
(the RedChannel version just iterate all clients).
Even if the channel would support multiple clients in this case
we would need to check only the one we are sending the message to.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Dispatcher is just storing the thead_id for MainDispatcher,
move thread_if to MainDispatcher to avoid useless field and API.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Distributions like to be in control of this.
From a patch from James Le Cuirot.
Instead of always build both shared and static libraries for
SPICE server (static is used for the tests) compile library
as user requested. In case we need the static library for tests
(which now can be disabled) create a static library extracting
objects from shared library.
This also fixes the problem that Meson installed the static
library even if not requested and just used for tests.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Remove this warning:
server/Makefile.am:5: warning: SUBDIRS was already defined in condition TRUE, which includes condition ENABLE_TESTS ...
server/Makefile.am:2: ... 'SUBDIRS' previously defined here
This was caused by the recent commit 38c7964d53
("build: Make building the test binaries optional under Autoconf")
This problems was also causing some issues as "noinst_PROGRAMS" target
were not compiled as expected.
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>
If file was compiled with DEBUG defined code did not compile.
Update code of the surrounded section.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
I've never seen used and Dispatcher is quite a code piece of
code, hard to break it.
I also tried to enable and it does not even compile anymore.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
We just created the object, we know is a RedCharDeviceSmartcard,
avoid to cast the pointer using RED_CHAR_DEVICE_SMARTCARD.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
These structures are not used that extensively, do not include
in the common header.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Not used in the header.
Also we reduced a lot the usage of these structures.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
red_channel_client_destroy is not called anymore from RedClient and
should not so update the comments.
red_channel_client_destroy, compared to other XXX_destroy functions
did not unreference the object but was just disconnecting the
client channel so use red_channel_client_disconnect instead.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
The object is already destroyed calling red_channel_disconnect_client.
The difference is that red_channel_disconnect_client does it in
the right thread while red_channel_client_destroy here attempts to
do potentially from another thread. This however at this stage
(after already being disconnected) is not an issue, just redundant
and potentially unsafe if red_channel_client_destroy change in
a way to have issue with multiple threads.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
The field is only set and never checked.
Cascading cleaning up red_channel_client_set_destroying.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
"RedChannelClient::on_disconnect", which for SpiceVMC is bound to
"spicevmc_red_channel_client_on_disconnect", is called while the
RedChannel object is disconnecting. More precisely is called after
the object has been removed from RedChannel causing
"red_channel_client_is_connected" to be false.
Calling "red_channel_client_destroy" this will lead
to a no-op as function will set "destroying" and call
"red_channel_client_disconnect" which will just exit finding
"red_channel_client_is_connected" false.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Minimal style change.
Allows to easier change the size of the buffer.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Francesco Giudici <fgiudici@redhat.com>
Minimal style change.
Allows to easier change the size of the buffer.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Francesco Giudici <fgiudici@redhat.com>
Allows to specify abstract Unix sockets addresses.
These Unix sockets are supported on Linux and allows to not
have file system names.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Kevin Pouget <kpouget@redhat.com>
It is not guaranteed that all the messages will be sent on a single
push call, unless this is done very early.
Although currently this function is called very early remove
this hidden assumption in the code.
"ping_id" is just incremented every time a message is sent to
client (in "main_channel_marshall_ping") so the formulae
"mcc->priv->ping_id - 2" was there to get the id of two messages
ago (the first message queued in this function) assuming all
messages were pushed at the time the formulae is used.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Snir Sheriber <ssheribe@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>
main_channel_client_push_ping is never called with mcc NULL.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Snir Sheriber <ssheribe@redhat.com>
Instead of freeing it in both reds_handle_main_link and
reds_handle_other_links free it in reds_handle_link.
This will reduce the chances code is changed adding a leak in some paths.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Julien Rope <jrope@redhat.com>
Instead of freeing manually the field and then detaching from
the structure just detach only if retained.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Due to aliasing using pointers lead to some sub-optimization.
Use local variable and then write them to output to improve performances.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Julien Rope <jrope@redhat.com>
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>
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>
Channel client creation can fail in some situation.
For instance if during a migration the client is disconnected.
In most cases this is ignored (this is usually logged in
red_channel_client_initable_init) but not in case of
CursorChannelClient and StreamChannelClient.
This fixes rhbz#1788757.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Julien Rope <jrope@redhat.com>
red_channel_client_shutdown remove the watch and shutdown the
socket. Reuse in red_channel_client_disconnect.
Calling shutdown will close the connection earlier.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
This information is retrieved multiple time.
Avoid to use just g_object_get which is dynamic so potentially
unsafe and slow.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
MainChannelClient is used by other clients to store some data
so should not disappear if other clients are still present.
Keep a owning reference to it and release after RedClient is
released.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Disconnecting a single channel from the client caused the server to
keep a stale channel client (RedChannelClient object) till the client
(RedClient object) entirely disconnected, that is the channel client
is disconnected but still in the list preventing new connections.
Calling red_client_remove_channel from red_channel_client_disconnect
fixes this last issue.
An issue was that was not clear how the ownership were managed. When
red_client_destroy was called red_channel_client_destroy was called
which freed the RedChannelClient object so this should imply
ownership.
However same red_channel_client_destroy call was attempted by
RedChannel using its list (red_channel_destroy). Basically the two
pointers (the one from the channel and the one from the client) were
considered as one ownership. To avoid the confusion now the client
list always decrement the counter.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
The channels list was not protected by a lock in red_client_destroy.
This could cause for instance a RedChannelClient object to be
created while scanning the list so potentially modifying the
list while scanning with all race issues.
Consider a client which attempt to connect to a new channel and
then for some reason close the main channel:
- the new channel connection is handled by the main thread;
- the relative RCC will be passed to red_channel_connect which
can decide to continue the initialization in another thread
(this happens currently for CursorChannelClient and
DisplayChannelClient);
- the main thread will catch the main channel disconnection and
call main_dispatcher_client_disconnect
(from main_channel_client_on_disconnect);
- main thread calls reds_client_disconnect;
- reds_client_disconnect calls red_client_destroy;
- now we have 2 thread which will attempt to change RedClient::channels
list, one protected by the mutex the other not.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Create some traffic on the connection to avoid potential timeout
on some proxies implementation which require some TCP data traffic.
The timeout used by default is quite big (5 minutes) to reduce network
traffic.
In case connectivity monitoring is enabled or latency monitor is
requested the timeout is reduced to the old default.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
This is a preparatory patch.
The "latency_monitor" feature allows to measure the latency of a
specific channel client.
Currently the measure is attempted every PING_TEST_TIMEOUT_MS which
is a constant.
To be able to use a different frequency allows to change this for every
channel client.
This feature will be also used to create some traffic on the connection
to allows some sort of keep-alive to overcome some proxy implementation
which requires some TCP data traffic.
This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1719736.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
"&rcc->priv->connectivity_monitor" is cached in "monitor" variable.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
These functions already check for NULL.
They are used mainly for cleanup, so cold path of code so speed
in case of NULL is not important (and usually should not be NULL).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@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>
OpenGL and Slirp requirement were removed much time ago.
OpenGL was removed by c5c176a5c7
(cfr "server: remove OpenGL", Set 2013).
Slirp was removed by ef9a8bf053
(cfr "Remove tunneling support", Oct 2013).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
The threading API for 1.0 was replaced with the THREADID API.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Missing headers for BN_ and RSA_ functions.
Initialization is deprecated with 1.1.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Frediano Ziglio <fziglio@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>
Put more event loop code in event-loop.c.
This is a preparation patch for the next one.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
The buffer could change inside smartcard_read_buf_prepare.
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>
This patch handles the scenario when a single read to guest device
brings multiple requests to be handled. When this happens, we will
iterate till all requests are handled and no more requests can be read
from guest device.
If the remaining buffer contains a full request we don't need to read
other bytes (note that there could be no bytes left), just parse the
request.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Use memmove instead of memcpy as the buffer can overlap if the second
request if bigger than the first.
"buf_pos" points to the point of the buffer after we read, if we want
the first part of the next request is "buf_pos - remaining".
Same consideration setting "buf_pos" for the next iteration.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Avoid client to trigger crash. The value of smartcard_readers_get
is checked for NULL so returning it it's not an issue.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
More coherent. Also it's not good for a library to output on
standard output.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
red_channel_client_handle_message is called after parsing the
message so it's not necessary to check it again or parse manually.
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>
The generated code handle possible endianess mismatch and check
for message format.
The copy back to "write_buf" allows to use that buffer to send
data back to device.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
"name" parameter of smartcard_channel_client_add_reader it's not
used.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
"self-tokens" property is 64 bit and must be passed as 64 bit on
32 bit machines to avoid memory corruptions.
This was introduced by 01de3b8922 ("spicevmc: Avoids DoS if
guest device is not able to get data faster enough"), detected by CI.
It caused this error (split into multiple lines):
(./test-leaks:15879): GLib-GObject-CRITICAL **: 14:03:59.650: \
g_object_new_is_valid_property: object class 'RedCharDeviceSpiceVmc' has \
no property named '\xb0/@\xf3\u0001'
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
This fix half (one direction) of
https://gitlab.freedesktop.org/spice/spice/issues/29.
Specifically if you have attempt to transfer a file from the client
using WebDAV.
Previously the queue to the device was unbound. If device was not
getting data fast enough the server started queuing data.
To easily test this you can suspend the WebDAV daemon while transferring
a big file from the client.
To simplify the code and reduce the changes server buffers are
used. This as client token implementation is written with agent
in mind. While when we exhaust server tokens RedCharDevice doesn't
close the client when we exhaust client tokens RedCharDevice closes
the client which in this case it's not wanted.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
If the client is keeping sending data while we can't handle them
(for instance because we need to forward to a device but the
device is not fast enough to receive that amount of data) allows
to stop RedChannelClient to read data.
This after a bit will stop the client sending data as its output
buffer will become full.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
This fix half (one direction) of
https://gitlab.freedesktop.org/spice/spice/issues/29.
Specifically if you have attempt to transfer a file to the client
using WebDAV.
Previously the queue to the client was unbound. If client was not
getting data fast enough the server started queuing data.
To easily test this you can use a tool to limit bandwidth and
transfer a big file (I used a "dd if=/dev/zero bs=1M of=test") from
the guest.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
As we don't use any token there's no reason to not queue directly instead
of passing through RedCharDevice.
This will make easier to limit the queue which currently is unlimited.
RedCharDevice flow control has some problems:
- it's really designed with VDI in mind. This for SpiceVMC would cause
code implementation to be confusing having to satisfy the agent token
handling;
- it's using items as unit not allowing counting bytes;
- it duplicates some queue management between RedChannelClient;
- it's broken (see comments in char-device.h);
- it forces "clients" to behave in some way not altering for instance the
queued items (which is very useful if items can be collapsed together);
- it replicates the token handling on the device queue too. This could
seems right but is not that if you have a network card going @ 1 GBit/s
you are able to upload more than 1 Gbit/s just using multiple
connections. The kernel will use a single queue for the network interface
limiting and sharing de facto the various buffers between the multiple
connections.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
It does not make sense to have a graphic card without a monitor.
In spice_qxl_set_max_monitors we prevent to set 0 monitors, do
the same in spice_qxl_set_device_info.
This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1691721.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Tested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
As now is an opaque type for RedCharDevice use the type that
better suits us.
This avoid useless conversions or look ups.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
RedClient was an opaque structure for RedCharDevice.
It started to be used when RedsState started to contain all
the global state.
Make it opaque again using a new RedCharDeviceClientOpaque.
The RedCharDeviceClientOpaque define in the header allows users
of the class to override the type to get a more safe type
than RedClient.
The define at the beginning of C file is to make sure we don't
use the opaque type as a specific one.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
No much reason for not inlining it, it's quite small and do
not reduce readability.
Note that spice_server_migrate_switch is deprecated and not
used by Qemu since commit 67be6726b6459472103ee87ceaf2e8e97c154965
(cfr "spice: raise requirement to 0.12" September 2012).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
This patch enables the SPICE_DISPLAY_CAP_PREF_VIDEO_CODEC_TYPE
capability for the stream-channel.
video_stream_parse_preferred_codecs: new function for parsing the
SPICE protocol message. This code used to in inside
dcc_handle_preferred_video_codec_type.
struct StreamChannelClient::client_preferred_video_codecs: new field.
Signed-off-by: Kevin Pouget <kpouget@redhat.com>
Acked-by: Frediano Ziglio <fziglio@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>
This is currently more style patch as the "value" field is the
first field of SpiceStatNode structure, so has offset 0. However
to compute the containing structure it better to use the proper
macro to avoid confusion.
If the offset won't be 0 the subtraction would compute the
wrong pointer as the offset is expressed in bytes but the
element size are uint64_t.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
spice_server_migrate_client_state was removed by
commit 3c6b4e415f
Author: Marc-André Lureau <marcandre.lureau@gmail.com>
Date: Fri Oct 24 17:16:35 2014 +0200
Remove spice-experimental
Remove unneded symbols that nobody should be using anyway.
ABI is modified with this patch, but the library version is not bumped.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
The GLZ code is basically LZ code (in spice-common) sharing image
segments between multiple images.
The code for RLE check in LZ (common/lz_compress_tmpl.c) is dealing
with both RLE and dictionary matches being:
if (!distance) {
/* zero distance means a run */
PIXEL x = *ref;
while ((ip < ip_bound) && (ref < ref_limit)) { // TODO: maybe separate a run from
// the same seg or from different
// ones in order to spare
// ref < ref_limit
in GLZ that part was copied to RLE part removing the need for the
second segment ("ref"). However the comment and setting ref variables
were not removed. Remove the old code to avoid confusions.
This also remove a Covscan warning as ref_limit was set not not used
(reported by Uri Lublin).
The warning was:
CLANG warning: "Value stored to 'ref_limit' is never read"
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
