When using setjmp/longjmp the state of local variables can be
undefined in certain scenarios:
[quote man(longjmp)]
The values of automatic variables are unspecified after a
call to longjmp() if they meet all the following criteria:
· they are local to the function that made the correspond‐
ing setjmp(3) call;
· their values are changed between the calls to setjmp(3)
and longjmp(); and
· they are not declared as volatile.
[/quote]
* server/red_worker.c: Mark some vars as volatile
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
* client/red_channel.cpp: AbortTrigger::on_event can't return
given its current impl
* server/red_worker.c: red_worker_main can't return
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
All printf var-args style methods should be annotation with
their format. All format strings must be const strings.
* client/application.cpp, client/cmd_line_parser.cpp,
client/hot_keys.cpp: Avoid non-const format
* client/client_net_socket.cpp: Fix broken format specifier
* client/red_peer.cpp: Fix missing format specifier
* client/platform.h: Add SPICE_GNUC_PRINTF annotation to term_printf
* client/utils.h: Add SPICE_GNUC_PRINTF annotation to string_printf
* server/glz_encoder_config.h, server/red_worker.c: Add
SPICE_GNUC_PRINTF annotation to warning callbacks
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
* server/red_worker.c: Add missing const for return type
* server/reds.c: Static strings must be declared const
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Arithmetic on void * types is non-portable & trivially avoided
* server/dispatcher.c: Use uint8_t for arithmetic
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Note, there may be line-ending issues here is we are not careful. This file is a msdos formatted file.
There was some inconsistencies in those line-ending that I have normalized.
Signed-off-by: Aric Stewart <aric@codeweavers.com>
This involves:
generating .c instead of .cpp files
Not including "common.h"
Standardizing some parameters
Signed-off-by: Aric Stewart <aric@codeweavers.com>
This patch changed getvirt to continue working even if spice_critical
doesn't abort (i.e. SPICE_ABORT_LEVEL != -1). This is in preparation to
make getvirt not abort at all. The reason is that getvirt is run on
guest provided memory, so a bad driver can crash the vm.
The hotkey parser was generating a string with sprintf and data it got
from the controller socket (ie data which can have an arbitrary
length). Using sprintf there is a bad idea, replace it with snprintf.
This patch will replace the common/ directory with the spice-common
project. It is for now a simple project subdirectory shared with
spice-gtk, but the goal is to make it a proper library later on.
With this change, the spice-server build is broken. The following
commits fix the build, and have been seperated to ease the review.
v2
- moves all the generated marshallers to spice-common library
- don't attempt to fix windows VS build, which should somehow be
splitted with spice-common (or built from tarball only to avoid
generation tools/libs deps)
v3
- uses libspice-common-client
- fix a mutex.h inclusion reported by Alon
cb_get_virt and cb_validate_virt have disappeared a long time ago,
not needed since:
commit 5ac88aa79f
Author: Gerd Hoffmann <kraxel@redhat.com>
Date: Thu Jul 1 17:55:33 2010 +0200
Properly parse QXLImage to the new-world SpiceImage
SpiceImage now replaces RedImage and has all image types in it.
All image data are now chunked (and as such not copied when demarshalling).
When we get the socket name for the foreign menu socket from
an environment variable, we cannot make assumptions about its length.
Currently, we are strncpying to a 50 byte buffer which is easily
too small.
The spice controller socket name used to be hardcoded to
/tmp/SpiceController-%lu.uds and generated using snprintf. A 50 bytes
buffer was enough for that, but this was changed in commit 79fffbf95
because this was predictable and allowed other users on the system
to sniff the browser/client communication. spicec now uses the value
of the environment SPICE_XPI_SOCKET as the name of the socket to use.
However, since the name that is used is no longer generated by spicec,
no assumption can be made about its size. Currently, the socket is
created inside the user home directory, which means that if the
user name is too long, spicec will not be able to read the controller
socket name.
This commit directly uses the string from getenv as he name of the
controller socket (on Linux) instead of limiting its size to 50 characters,
which should fix this issue.
This fixes rhbz #804561
If we run out of watches slots, we return NULL from watch_add, which
means that the other watch_foo functions may get called with a NULL
parameter, protect them against this.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
If we allow listening on arbitrary sockets like unix sockets,
we can get ENOPROTOOPT errors from setsockopt calls that set TCP
specific options. This should be allowed to happen.
Commit 143a1df24e changed red_worker_main
from epoll to poll. But epoll has edge triggered semantics (when requested
and we requested them), where as poll is always level triggered. And
red_worker was relying on the edge triggered semantics, as it was always
polling for POLLOUT, which, when edge triggered, would only cause poll
to register an event after we had blocked on a write. But after the
switch to regular poll, with its level triggered semantics, the POLLOUT
condition would almost always be true, causing red_worker_main to not
block on the poll and burn CPU as fast as it can as soon as a client was
connected.
Luckily we already have a mechanism to switch from polling for read only
to polling for read+write and back again in the form of watches. So this
patch changes the red_worker dummy watch implementation into a proper watch
implementation, and drops the entire EventListener concept since that then is
no longer needed.
This fixes spice-server using 400% CPU on my quad core machine as soon as
a client was connected to a multi head vm, and as an added bonus is a nice
cleanup IMHO.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
We allow channels to have different core implementations, but we were
relying on reds_stream_free to remove the stream watch on disconnect,
and reds_stream_free always uses the qemu core implementation.
So far we were getting away with this since all the alternative core
implementations always return NULL from watch_add.
But:
1) The code before this patch clearly was not correct, since it was matching
a channel-core watch_add with a qemu-core watch_remove
2) I plan to move red_worker over to actually using an alternative watch
implementation at which point this becomes a real problem
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
The red_worker EventListener struct is either embedded in one of:
1) DisplayChannelClient
2) CursorChannelClient
3) RedWorker
And as such gets destroyed when these get destroyed, in case 1 & 2 through
a call to red_channel_client_destroy().
So free-ing it when the ref-count becomes 0 is wrong, for cases:
1) and 2) this will lead to a double free;
3) this will lead to passing memory to free which was not returned by malloc.
This is not causing any issues as the ref-count never gets decremented, other
then in red_worker_main where it gets incremented before it gets decremented,
so it never becomes 0.
So we might just as well completely remove it.
Notes:
1) This is mainly a preparation patch for fixing issues introduced by
the move from epoll to poll
2) Since removing the ref-counting removes the one code path where listeners
would get set to NULL, this patch moves the setting of NULL to
pre_disconnect, where it should have been done in the first place since
red_client_destroy calls red_channel_client_disconnect
(through the dispatcher) followed by red_channel_client_destroy, so
after pre_disconnect the listener may be gone.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
While git-bisecting another issue I ended up hitting and not recognizing
the bug fixed by commit 7a079b452b.
While fixing this (again) I noticed that (even after the fix) not all
users of ChannelCbs first zero it. So this patch ensures that all users of
ChannelCbs first zero it, and does the same for ClientCbs while at it.
Since before this patch there were multiple zero-ing styles, some using
memset and other using a zero initializer this patch also unifies all
the zero-ing to use a NULL initializer for the first element.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
This fixes a core dumped observed once by repeated migration. So far 100
migrations and no recurrence.
Core was generated by `/home/alon/spice/upstream/bin/qemu-system-x86_64 --enable-kvm -qmp unix:/tmp/mi'.
Program terminated with signal 11, Segmentation fault.
11197 if (evt_listener && evt_listener->refs > 1) {
Missing separate debuginfos, use: debuginfo-install bluez-libs-4.98-3.fc17.x86_64 brlapi-0.5.6-4.fc17.x86_64 bzip2-libs-1.0.6-4.fc17.x86_64 cryptopp-5.6.1-6.fc17.x86_64 keyutils-libs-1.5.5-2.fc17.x86_64 libssh2-1.4.0-1.fc17.x86_64 nss-softokn-freebl-3.13.1-20.fc17.x86_64 xen-libs-4.1.2-11.fc17.x86_64 xz-libs-5.1.1-2alpha.fc17.x86_64
(gdb) bt
(gdb) l
11192 for (i = 0; i < MAX_EVENT_SOURCES; i++) {
11193 struct pollfd *pfd = worker.poll_fds + i;
11194 if (pfd->revents) {
11195 EventListener *evt_listener = worker.listeners[i];
11196
11197 if (evt_listener && evt_listener->refs > 1) {
11198 evt_listener->action(evt_listener, pfd);
11199 if (--evt_listener->refs) {
11200 continue;
11201 }
(gdb) p evt_listener
$1 = (EventListener *) 0x7f15a9a5d1e0
(gdb) p *evt_listener
Cannot access memory at address 0x7f15a9a5d1e0
(gdb) p i
$2 = 2
(gdb) p worker.listeners
$3 = {0x7f15bc832520, 0x7f15a406e1a0, 0x7f15a9a5d1e0, 0x0 <repeats 17 times>}
If specified, use SPICE_FOREIGN_MENU_SOCKET environment variable over
snprintf(pipe_name, PIPE_NAME_MAX_LEN, PIPE_NAME,
Platform::get_process_id()); since it's impossible to guess the client
pid when spice-xpi launches the client as a grand-child (via script
etc).
This is also more aligned with the way we handle SPICE_XPI_SOCKET
location.
Add spice_server_set_name() and spice_server_set_uuid() that allows
the client to identify a Spice server (useful to associate settings
with a particular server)
The SPICE_MSG_MAIN_NAME and SPICE_MSG_MAIN_UUID messages are only sent
to capable clients, announcing SPICE_MAIN_CAP_NAME_AND_UUID.
A message with a fixed-size array, such as uint8 uuid[16] will
generate an invalid code, missing the __nelements variable. Make sure
that variable is defined.
