Provide constructor and destructor, use new/delete.
This allows additional fields to be properly initialized if needed.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
Remove some conditional compilation.
Make try_compress_lz4 return success or failure and change the
messages making easier to read result from try_compress_lz4.
Remove "n" parameter from try_compress_lz4, use message size
instead.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
Reduces usage of manual reference counting.
Avoids usage of new and use instead red::make_shared to both
create and own pipe items.
Use move semantic to reduce useless copies.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
Now that items are managed by constructors/destructors avoid some
manual releases.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
Provides a base class to allows RedCharDeviceVDIPort to pass
a RedPipeItem to MainChannelClient instead of having to wrap
into another item.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
Most of the times each class has an associated constant so
allows to be able to define the constant at declaration time.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
This allows to:
- reuse reference counting;
- avoid having to manually call g_free to release item memory;
- assure item is initialized;
- avoids some manual casts.
It will also allows to use smart pointers.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
To avoid memory errors marshaller_unref_pipe_item should be
matched with a previous call to red_pipe_item_ref.
This is correctly done but to reduce the possibility of breaking
this rule move both referencing and unreferencing into a new
RedPipeItem::add_to_marshaller method.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
Exit more cleanly, otherwise sometimes some strange failures
occurs. This is due to the fact that a thread is cleaning up
everything while the others are still working.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Uri Lublin <ulublin@redhat.com>
RedCharDeviceSmartcard is not using token handling (which is designed
only for VDAgent) so push directly to client.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
For historical reasons this was computed using VDAgent details.
Provide a more suitable and simple constant.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Uri Lublin <ulublin@redhat.com>
"password" is pretty small, allocate on stack directly.
RSA is currently 1024 bits, that is more or less 128 bytes.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Kevin Pouget <kpouget@redhat.com>
This flag allows to catch variables or arguments hiding other
variables or attributes.
It helps avoiding some possible mistakes.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
This is a regression of commit 176970f3f1
(cfr "red-channel-client: Remove GObject type").
Attribute was not initialised.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
Move to Doxygen (output was not meant to generate output
automatically).
Update some part that were outdated.
Typo messsage_type -> message_type.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
Allows to simplify a bit marshalling code in both CursorChannel
and DisplayChannel as they use the same marshalling structure.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
The pipe items are meant to be used by channel clients, so move
declaration where channel clients is.
This item is used by both CursorChannelClient and DisplayChannelClient
so the CommonGraphicsChannel is the proper place.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
RedCacheItem was using an union to reuse cache item memory
as a pipe item to release that specific cache item.
Instead of spreading that structure everywhere move the specific
optimization all in cache-item.tmpl.cpp.
This make also code less cluttered.
Add some comment on specific code.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
Not much used, can be retrieved from the class if needed.
There are some apparent test removal on reds.cpp.
The test "if (!reds->vdagent)" apparently disappeared but it's included in
the while.
reds->agent_dev cannot be NULL as SPICE server allows only one agent and we
are into a member of an object so it must be reds->agent_dev.get() == this.
As there's only an agent and as the interface (sin) is extracted from that
agent it must be reds->agent_dev.get() == sin->st and either reds->vdagent
== sin or reds->vdagent == NULL.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
Avoid to pass throught SpiceCharDeviceInterface.
When data is read from device the device get marked as active.
This is coherent with what is done in red_char_device_read_from_device.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
Just pass rest of the arguments.
Removes some extra semicolon.
This change also remove a specific GCC extension.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
The handling of that specific pipe item reference counting
was a bit confusing. In particular every time before adding to
the queue the item was reset causing the reference counter to
be 1 however it was not sure that the item was not still referenced
when added again to the queue.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
The assumption is a bit fragile as code could change adding
additional references to the pipe item.
Instead use marshaller cleanup routines, this surely will be
triggered when the message is queued to the network.
This also causes some additional cleanups.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
It is called from 2 places.
1. display-channel.cpp:
dcc_get_encoders always returns a non-null pointer.
2. image-encoders.cpp: image_encoders_release_glz
<- image_encoders_free (where enc is not checked)
<- dcc.c:image_encoders_free
where non-null pointer is passed.
Signed-off-by: Uri Lublin <uril@redhat.com>
socket_close is mapped to close under Unix but under Windows
you should call closesocket instead so call socket_close which
will map to the proper close function on both environments.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Uri Lublin <ulublin@redhat.com>
__glz_dictionary_window_alloc_image may return NULL.
Check that the image was created successfully and if not
then error out using dict->cur_usr->error like it's done
few lines below.
Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
glz_enc_dictionary_create may return NULL.
glz_enc_dictionary_restore itself may return NULL so
add one more check.
Found-by: gcc (10) analyzer
Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
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>
