Python 3 puts its .pyc files in a __pycache__ subdirectory, This commit
adds these to DISTCLEANFILES. My main motivation for doing this is so
that git.mk can then properly add them to .gitignore.
Acked-by: Victor Toso <victortoso@redhat.com>
It was added together with the meson build, but it's currently being
managed by git.mk, so it should not be in git
Acked-by: Victor Toso <victortoso@redhat.com>
Nothing in spice-common calls vfork. The autotools-based build can
fallback to vfork if fork is not available/working through the
AC_FUNC_FORK macro, but the meson build is not implementing this magic.
However, fork() is only called once in backtrace.h, and this part of the
code is optional (it's not compiled in on Windows for example), and
anyway, I doubt anyone is going to try to compile SPICE code on a
platform without fork, so we can remove this check, it's always possible
to readd it when we have a clear bug report about what is
missing/needed.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
At the moment, missing optional dependencies will be silently ignored
when the corresponding configuration flag is 'true', and won't be tested
for when the flag is 'false'.
In the autotools build, this corresponds respectively to
--enable-foo=auto and --disable-foo.
Having a way to get an error when the package is enabled but missing is
quite desirable. f52247384 has some half-baked attempt at that, but this
was supposed to be removed from the series. This causes errors when
processing meson.build, breaking the meson build.
This commit adds 'true'/'false'/'auto' in a proper way.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Different processors has specific instructions to count leading
zero bits. This includes: x86. x64, arm, ppc.
For portability reason the behaviour of __builtin_clz is not
defined if the value is zero so test for it.
Currently the function is not called with the value or 0.
This increase performance decoding of about 4-5% on a x64 machine
(code size decreases a little too, but about 0.1%).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
These sources are only used by the client, so they do not belong in
libspice_common_la_SOURCES.
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Grepping for 'pow', 'sqrt' or 'inet_ntoa' returns no results in
spice-common code base.
inet_ntoa use was removed in 9749e7e 'ssl-verify: Changed IPv4 hostname
to IPv6' and pow/sqrt use in 384698a 'Remove GL support'
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
AM_COND_IF can be replaced in constructs like:
AM_CONDITIONAL([HAVE_OPUS], [test "x$have_opus" = "xyes"])
if test "x$have_opus" = "xyes" ; then
AC_DEFINE([HAVE_OPUS], [1], [Define if we have OPUS])
fi
which becomes:
AM_CONDITIONAL([HAVE_OPUS], [test "x$have_opus" = "xyes"])
AM_COND_IF([HAVE_OPUS], AC_DEFINE([HAVE_OPUS], [1], [Define if we have OPUS]))
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Following the commit disabling celt by default, it's quite easy to have
a build without both celt and opus. After this commit, Opus will have to
be installed for a successful build unless one passes --disable-opus.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This version of the CELT codec has long been obsolete, and Opus support
has been added nearly 5 years ago. It's time we move on and try to stop
using Celt ;)
This commit disables CELT by default, but since this could be an
unexpected change for packagers, if CELT 0.5.1 development headers are
installed, it will error out unless --enable-celt051/--disable-celt051
has been explicitly specified.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
lines.c uses hypot(), which is found in libm on some systems. This means
we are currently relying on getting -lm indirectly through some other
means, as otherwise linking any binary with spice-common would fail.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
The image is going to network and network protocol is little endian
so the numbers has to be little endian. Note that this is already done
during decoding.
Tested on a ppc64 machine.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@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>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
../tests/test-quic.c: In function ‘gdk_pixbuf_new_random’:
../tests/test-quic.c:205:19: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
for (i = 0; i < gdk_pixbuf_get_byte_length(random_pixbuf); i++) {
^
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Some assert in the code are doing some paranoid test and in code
paths quite hot.
The encoding time is reduced by 30-50% while the decoding time
is reduced by a 20-30%.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
The quic code goes through a function pointer in two places in order to
try to prevent the compiler from inlining code.
Doing performance measurements this trick does not work anymore
and just make code less readable.
This patch and message was based on a previous work of
Christophe Fergeau.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
This only adds a basic test relying on gdk-pixbuf.
The main limitation is that gdk-pixbuf does not handle 16bpp images,
nor 32bpp/no alpha images. I should have picked something else instead ;)
This allows at least to exercise the QUIC_IMAGE_TYPE_RGB24 and
QUIC_IMAGE_TYPE_RGBA codepaths.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
With last changes are just used once and are straight forward.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: jonathon Jongsma <jjongsma@redhat.com>
Do not extract all components and compare one by one, can be easily
compared together.
Performance measurements on a set of 16 bit images shown an improve of
about 10%.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This avoids the need for a local variable with the right name (which
was decorrelate_drow in some cases in quic_tmpl.c...)
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
'correlation_row' is always set to channel->colleration_row, and we
already pass 'channel' as an argument.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
The naming is odd as this is just an alias for channel->correlate_row.
This will also help in subsequent commits to make things more
consistent with quic_rgb_tmpl.c
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Most functions in quic_tmpl.c/quic_rgb_tmpl.c have only superficial
differences. One of them is using channel->state or encoder->rgb_state.
This commit adds a local CommonState *state in all template methods
which will be used instead of channel->state or encoder->rgb_state.
This makes it easier to spot the common code between the 2 template
files...
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
We don't need 2 different implementations when the only difference is
the CommonState which is being used.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
It's always set, no need to have conditional compilation based on it.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
It's always set, no need to have conditional compilation based on it.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
It's always set, no need to have conditional compilation based on it.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
It's hardcoded at compile-time, and I don't think it was changed in
years...
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
It's hardcoded at compile-time, and I don't think it was changed in
years...
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
test-logging makes use of functions only available from this version
../tests/test-logging.c: In function ‘test_spice_abort_level’:
../tests/test-logging.c:50:5: error: ‘g_test_subprocess’ is deprecated: Not available before 2.38 [-Werror=deprecated-declarations]
if (g_test_subprocess()) {
^~
In file included from /usr/include/glib-2.0/glib.h:82:0,
from ../tests/test-logging.c:23:
/usr/include/glib-2.0/glib/gtestutils.h:151:10: note: declared here
gboolean g_test_subprocess (void);
^~~~~~~~~~~~~~~~~
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Make sure there are not 2 messages with the same value in the
same channel.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Make sure there are not 2 messages with the same name in the
same channel.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Code that handled client and server messages check was the same, just
changed some variable names.
Instead use a class to store same information and reuse the code.
This allows easier extension of the 2 path of code.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Lukáš Hrázký <lhrazky@redhat.com>
This is the only function starting with an underscore, looks
out of style.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Building with gcc 8.0.1 from Fedora 28 gives the following error:
FAILED: common/common@@spice-common@sta/marshaller.c.o
../common/marshaller.c: In function 'spice_marshaller_reserve_space':
../common/marshaller.c:311:27: error: cast between incompatible function types from 'void (*)(void *)' to 'void (*)(uint8_t *, void *)' {aka 'void (*)(unsigned char *, void *)'} [-Werror=cast-function-type]
item->free_data = (spice_marshaller_item_free_func)free;
^
cc1: all warnings being treated as errors
Which can be easily fixed by creating a new function with the correct
signature and calling free() from it.
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Rename struct VSCMsgReaderAdd field 'reader_name' to 'name', and struct
VSCMsgATR field 'data' to 'atr' to match their definitions in file
vscard_common.h.
The error log follows:
generated_server_demarshallers.c:1985:30: note: each undeclared identifier is reported only once for each function it appears in
generated_server_demarshallers.c:1994:15: error: ‘VSCMsgReaderAdd {aka struct VSCMsgReaderAdd}’ has no member named ‘reader_name’
memcpy(out->reader_name, in, reader_name__nelements);
^~
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Even though commit df4ec5c318 commented
out most of smartcard code which triggered this error, it still might
happen if a new message is added with an array member.
The reason is a missing declaration of mem_size, which is fixed simply
by checking if the attribute 'nocopy' is present.
The error log follows:
generated_server_demarshallers.c: In function ‘parse_msgc_smartcard_reader_add’:
generated_server_demarshallers.c:1985:30: error: ‘mem_size’ undeclared (first use in this function); did you mean ‘nw_size’?
data = (uint8_t *)malloc(mem_size);
^~~~~~~~
nw_size
This patch also updates test-marshallers so that this bug is triggered.
The diff between generated demarshallers with the patch applied follows:
--- tests/generated_test_demarshallers.c.old 2018-05-17 14:35:29.234056487 -0300
+++ tests/generated_test_demarshallers.c 2018-05-17 14:35:40.554031295 -0300
@@ -286,6 +286,7 @@ static uint8_t * parse_msg_main_ArrayMes
uint8_t *start = message_start;
uint8_t *data = NULL;
uint64_t nw_size;
+ uint64_t mem_size;
uint8_t *in, *end;
uint64_t name__nw_size;
uint64_t name__nelements;
@@ -298,6 +299,7 @@ static uint8_t * parse_msg_main_ArrayMes
}
nw_size = 0 + name__nw_size;
+ mem_size = sizeof(SpiceMsgMainArrayMessage);
/* Check if message fits in reported side */
if (nw_size > (uintptr_t) (message_end - start)) {
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This causes errors if Valgrind or sanitizer or similar memory
leak checkers are used.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
This hack is now made obsolete by the previous commit. We can safely
remove those defines and the code now builds fine for both spice server
and spice-gtk.
commit df4ec5c318
Author: Frediano Ziglio <fziglio@redhat.com>
Date: Fri May 11 16:59:46 2018 +0100
Fix generation of Smartcard channel
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This small test prove a that current generated demarshaller code
is not safe to integer overflows leading to buffer overflows.
Actually from a quick look at the protocol it seems that client
can't cause these overflows but server can quite easily at
demonstrated by this test.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Make code safe using both 32 and 64 bit machine.
Consider that this code can be compiled for machines with 32 bit.
There are some arrays length which are 32 bit.
If size_t this can cause easily an overflow. For instance message_len
sending SPICE_MSG_NOTIFY messages are 32 bit and code add a small
constant (currently 24) before doing the test for size. Now passing
(uint32_t) -20 as message_len would lead to a size of 4 after the
addition. This overflow does not happen on 64 bit machine as the length
is converted to size_t.
There are also some array length where some item are bigger than 1 byte.
For instance SPICE_MAIN_CHANNELS_LIST message have a number of channels
and each channel is composed by 2 bytes. Now the code generated try to do
length * 2 where length is still a 32 bit so if we put a value like
0x80000002u we get 4 as length. This will cause an overflow as code will
allocate very few bytes but try to fill with a huge number of elements.
This overflow happen in both 32 and 64 bit machine.
To avoid all these possible overflows this patch use only 64 bit for
nelements (number of elements), nw_size (network size) and mem_size
(memory size needed) checking the sizes to avoid other overflows
(like pointers conversions under 32 bit machines).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe de Dinechin <dinechin@redhat.com>
The macro for both depth is the same, reuse the definition.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
This change does not affect generated code but make source more
readable. Also document in a single location the range of this
type.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
When compiling spice-common with meson/ninja under "release" mode, I get
several compiler warnings about possibly-uninitialized members. For
example:
../subprojects/spice-common/common/lines.c: In function ‘miLineArc’:
../subprojects/spice-common/common/lines.c:2167:17: error: ‘edge2.dx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
edge->e += edge->dx; \
^~
../subprojects/spice-common/common/lines.c:2426:24: note: ‘edge2.dx’ was declared here
PolyEdgeRec edge1, edge2;
^~~~~
Initializing these structures to zero silences the warnings.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Christophe Fergeau