We require GLib 2.50 but G_SOURCE_FUNC was introduced in version
2.58. Do the conversion instead of using G_SOURCE_FUNC macro.

Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Victor Toso <victortoso@redhat.com>

On containers dbus could be not running.
In this case dbus.system_connection is NULL and calling
dbus_connection_close on it will cause a crash.
This happens also under Gitlab CI.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>

Check for spice/linux/vd_agent#18

.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>

Jakub Janků authored 5 years ago and Frediano Ziglio committed 5 years ago

Otherwise the caller doesn't know that the init failed
because we're returning 0 in the parent and 1 in child.

Signed-off-by: Jakub Janků <jjanku@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>

Jakub Janků authored 5 years ago and Frediano Ziglio committed 5 years ago

Creating threads is not compatible with forking as only the thread
that calls fork() is inherited.

Handlers registered with g_unix_signal_add() create a thread so
move these calls after fork.

Also call g_socket_service_start() after fork to avoid creation of
new threads before it is necessary.

Fixes: spice/linux/vd_agent#18

Also see: https://gitlab.gnome.org/GNOME/glib/issues/2073



Signed-off-by: Jakub Janků <jjanku@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>

Signed-off-by: Victor Toso <victortoso@redhat.com>

It was dropped in git master.

Signed-off-by: Victor Toso <victortoso@redhat.com>

Acked-by: Victor Toso <victortoso@redhat.com>

Divide creation of udscs_server into 2 steps:

1) udscs_server_new()
 - allocates new udscs_server struct and inits it

2) udscs_server_listen_to_fd(), udscs_server_listen_to_address()
 - starts accepting new connections

Remove udscs_create_server() and udscs_create_server_for_fd().

This makes the code a bit shorter and nicer.

Signed-off-by: Jakub Janků <jjanku@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>

Use G_DECLARE_FINAL_TYPE().

Rename
struct vdagent_virtio_port --> VirtioPort

Signed-off-by: Jakub Janků <jjanku@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>

Use G_DECLARE_FINAL_TYPE().

Rename
struct udscs_connection --> UdscsConnection

Signed-off-by: Jakub Janků <jjanku@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>

1) VDAgentConnection

Add vdagent-connection.{c,h} files,
define a new derivable class VDAgentConnection.

VDAgentConnection uses GIO and therefore integrates well with GMainLoop.

Read messages must begin with a header of a fixed size.
Message body size can vary.

Note:
vdagent_connection_destroy() cancels ongoing I/O-ops
and closes the underlying FD.
However, the I/O-ops do not finish immediately,
but asynchronously in the next iteration of the main loop.
At the beginning of each op, VDAgentConnection is referenced,
so the object isn't finalized until the ops themselves finish.
To solve this, call g_main_context_iteration() before the agent exits.

2) udscs

Make udscs_connection a subclass of VDAgentConnection.

Rewrite udscs_server using GSocketService.
Use GList to store server's connections.

Drop support for select(), remove:
 * udscs_server_fill_fds()
 * udscs_server_handle_fds()

Remove udscs_{set,get}_user_data(). Custom data can be
associated with the GObject using g_object_set_data().

3) virtio_port

Make vdagent_virtio_port a subclass of VDAgentConnection.

Drop support for select(), remove:
 * vdagent_virtio_port_fill_fds()
 * vdagent_virtio_port_handle_fds()

4) vdagentd

Replace the main_loop() with a GMainLoop.

Use g_unix_signal_add() to handle SIGINT, SIGHUP, SIGTERM.
SIGQUIT handling is not supported by GLib.

Integrate the session_info into the loop using
GIOChannel and g_io_add_watch().

Signed-off-by: Jakub Janků <jjanku@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>

Require UNIX-specific GIO package to build spice-vdagent.

This package includes:

- GDBus which is going to be used instead of libdbus in
  console-kit.c and systemd-login.c

- I/O stream classes and networking APIs that are going to
  be used in VDAgentConnection

Version 2.44 is required because of the following definitions:
- g_input_stream_read_all_*
- g_output_stream_write_all_*
- GSimpleIOStream

GIO requires GLib and GObject, so we can depend solely on it.

GLib version across different distributions:
    Fedora 30:    2.60
    Fedora 29:    2.58
    Fedora 28:    2.56

    RHEL 7.6:     2.56
    RHEL 7.5:     2.54
    RHEL 7.4:     2.50

    Debian 10:    2.58
    Debian 9:     2.50
    Debian 8:     2.42

    Ubuntu 19.04: 2.60
    Ubuntu 18.04: 2.56
    Ubuntu 16.04: 2.48

Signed-off-by: Jakub Janků <jjanku@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>

Implement the behaviour described in protocol "vdagent: introduce
VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL".

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>

Do not send a release event between two grabs, this helps with window
manager interaction issues on peer side.

Advertise this behaviour via a capability introduced in spice-protocol
0.12.16, so the client doesn't need to do some time-based filtering.

(the capability shouldn't need to be negotiated, a client shouldn't
expect a release between two grabs)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>

Do not rely on internal state when receiving clipboard events, as this
may race with external events. Use gtk_clipboard_set_with_owner()

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>

This will allow easier lifecycle management,
and usage of gtk_clipboard_set_with_owner()

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>

Improve readability a bit, reindent slightly.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>

This is required for using the new GObject macros.

According to commit 61fc548f ("Bump
GLib version to 2.34"), RHEL6 is no longer supported.

GLib version across some distributions, from repology:
- Debian Stable (9): 2.50.3
- CentOS 7: 2.56.1
- Fedora 26: 2.52.3 (fwiw, Fedora 30: 2.60.0)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>

The following patches are going to introduce GObject.  It's not a big
change, since usually glib comes installed with gobject on most systems.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>

Modernize a bit the code, using gtk_clipboard_get_selection().

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>

According to repology,
- Debian Stable (9): 3.22.11
- CentOS 7: 3.22.30
- Fedora 26: 3.22.21 (fwiw, Fedora 30: 3.24.7)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>

New GNOME versions have started to manage the session using the systemd
user instance. The effect of this is that the spice-vdagent running in
the user session is forked off (indirectly) from the systemd user
instance and does technically not belong to any session.

The correct way of handling this situation is to simply assume that the
process belongs to the users graphical session. Add a heuristic to find
the graphical session based on the UID, fixing spice-vdagent running on
GNOME 3.34 with systemd integration.

Acked-by: Victor Toso <victortoso@redhat.com>

Uri Lublin authored 5 years ago and Frediano Ziglio committed 5 years ago

A comment and curly brackets for style were added.

Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>

Previous commit set last element of orig_argv array to NULL.
That's redundant as g_memdup() uses memcpy() and it would do
just that. Add a comment that the reason for this change is
to workaround clang's warning.

Suggested by Uri.

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>

Otherwise we get a CLANG_WARNING due accessing garbage.

Covscan report:
 > spice-vdagent-0.19.0/src/vdagent/vdagent.c:471:9: warning: 1st function call argument is an uninitialized value
 > #        execvp(orig_argv[0], orig_argv);
 > #        ^      ~~~~~~~~~~~~
 > spice-vdagent-0.19.0/src/vdagent/vdagent.c:421:24: note: Storing uninitialized value
 > #    char **orig_argv = g_memdup(argv, sizeof(char*) * (argc+1));
 > #                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 > spice-vdagent-0.19.0/src/vdagent/vdagent.c:434:9: note: Assuming 'error' is equal to NULL
 > #    if (error != NULL) {
 > #        ^~~~~~~~~~~~~
 > spice-vdagent-0.19.0/src/vdagent/vdagent.c:434:5: note: Taking false branch
 > #    if (error != NULL) {
 > #    ^
 > spice-vdagent-0.19.0/src/vdagent/vdagent.c:442:9: note: Assuming 'portdev' is not equal to NULL
 > #    if (portdev == NULL)
 > #        ^~~~~~~~~~~~~~~
 > spice-vdagent-0.19.0/src/vdagent/vdagent.c:442:5: note: Taking false branch
 > #    if (portdev == NULL)
 > #    ^
 > spice-vdagent-0.19.0/src/vdagent/vdagent.c:445:9: note: Assuming 'vdagentd_socket' is not equal to NULL
 > #    if (vdagentd_socket == NULL)
 > #        ^~~~~~~~~~~~~~~~~~~~~~~
 > spice-vdagent-0.19.0/src/vdagent/vdagent.c:445:5: note: Taking false branch
 > #    if (vdagentd_socket == NULL)
 > #    ^
 > spice-vdagent-0.19.0/src/vdagent/vdagent.c:448:30: note: Assuming 'do_daemonize' is 0
 > #    openlog("spice-vdagent", do_daemonize ? LOG_PID : (LOG_PID | LOG_PERROR),
 > #                             ^~~~~~~~~~~~
 > spice-vdagent-0.19.0/src/vdagent/vdagent.c:448:30: note: '?' condition is false
 > spice-vdagent-0.19.0/src/vdagent/vdagent.c:451:9: note: Assuming the condition is false
 > #    if (!g_file_test(portdev, G_FILE_TEST_EXISTS)) {
 > #        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 > spice-vdagent-0.19.0/src/vdagent/vdagent.c:451:5: note: Taking false branch
 > #    if (!g_file_test(portdev, G_FILE_TEST_EXISTS)) {
 > #    ^
 > spice-vdagent-0.19.0/src/vdagent/vdagent.c:457:9: note: Assuming 'do_daemonize' is 0
 > #    if (do_daemonize)
 > #        ^~~~~~~~~~~~
 > spice-vdagent-0.19.0/src/vdagent/vdagent.c:457:5: note: Taking false branch
 > #    if (do_daemonize)
 > #    ^
 > spice-vdagent-0.19.0/src/vdagent/vdagent.c:468:9: note: Assuming 'version_mismatch' is not equal to 0
 > #    if (version_mismatch) {
 > #        ^~~~~~~~~~~~~~~~
 > spice-vdagent-0.19.0/src/vdagent/vdagent.c:468:5: note: Taking true branch
 > #    if (version_mismatch) {
 > #    ^
 > spice-vdagent-0.19.0/src/vdagent/vdagent.c:471:9: note: 1st function call argument is an uninitialized value
 > #        execvp(orig_argv[0], orig_argv);
 > #        ^      ~~~~~~~~~~~~
 > #  469|           syslog(LOG_INFO, "Version mismatch, restarting");
 > #  470|           sleep(1);
 > #  471|->         execvp(orig_argv[0], orig_argv);
 > #  472|       }
 > #  473|

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>

This is a CLANG_WARNING found by covscan. It is a false positive as
g_clear_pointer() does set vportp to NULL, meaning that the situation
described by covscan below should not be reached. Moving away from
g_clear_pointer() in this specific case just to make our tool happy.

Covscan report:
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:458:9: warning: Use of memory after it is freed
 > #    if (wbuf->write_pos != wbuf->size) {
 > #        ^
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:268:12: note: Assuming the condition is true
 > #    while (*vportp && (*vportp)->write_buf)
 > #           ^~~~~~~
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:268:12: note: Left side of '&&' is true
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:268:5: note: Loop condition is true.  Entering loop body
 > #    while (*vportp && (*vportp)->write_buf)
 > #    ^
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:269:9: note: Calling 'vdagent_virtio_port_do_write'
 > #        vdagent_virtio_port_do_write(vportp);
 > #        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:453:5: note: Taking false branch
 > #    if (!wbuf) {
 > #    ^
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:458:9: note: Assuming the condition is false
 > #    if (wbuf->write_pos != wbuf->size) {
 > #        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:458:5: note: Taking false branch
 > #    if (wbuf->write_pos != wbuf->size) {
 > #    ^
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:465:9: note: Assuming 'n' is < 0
 > #    if (n < 0) {
 > #        ^~~~~
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:465:5: note: Taking true branch
 > #    if (n < 0) {
 > #    ^
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:466:13: note: Assuming the condition is false
 > #        if (errno == EINTR)
 > #            ^~~~~~~~~~~~~~
 > /usr/include/errno.h:38:16: note: expanded from macro 'errno'
 > ## define errno (*__errno_location ())
 > #               ^
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:466:9: note: Taking false branch
 > #        if (errno == EINTR)
 > #        ^
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:469:9: note: Calling 'vdagent_virtio_port_destroy'
 > #        vdagent_virtio_port_destroy(vportp);
 > #        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:130:5: note: Taking false branch
 > #    if (!vport)
 > #    ^
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:133:9: note: Assuming the condition is false
 > #    if (vport->disconnect_callback)
 > #        ^~~~~~~~~~~~~~~~~~~~~~~~~~
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:133:5: note: Taking false branch
 > #    if (vport->disconnect_callback)
 > #    ^
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:137:5: note: Loop condition is true.  Entering loop body
 > #    while (wbuf) {
 > #    ^
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:140:9: note: Memory is released
 > #        g_free(wbuf);
 > #        ^~~~~~~~~~~~
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:137:5: note: Loop condition is false. Execution continues on line 144
 > #    while (wbuf) {
 > #    ^
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:144:5: note: Loop condition is true.  Entering loop body
 > #    for (i = 0; i < VDP_END_PORT; i++) {
 > #    ^
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:144:5: note: Loop condition is true.  Entering loop body
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:144:5: note: Loop condition is true.  Entering loop body
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:144:5: note: Loop condition is false. Execution continues on line 148
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:149:5: note: Assuming '_p' is null
 > #    g_clear_pointer(vportp, g_free);
 > #    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 > /usr/include/glib-2.0/glib/gmem.h:124:9: note: expanded from macro 'g_clear_pointer'
 > #    if (_p)                                                                    \
 > #        ^~
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:149:5: note: Taking false branch
 > /usr/include/glib-2.0/glib/gmem.h:124:5: note: expanded from macro 'g_clear_pointer'
 > #    if (_p)                                                                    \
 > #    ^
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:149:5: note: Loop condition is false.  Exiting loop
 > /usr/include/glib-2.0/glib/gmem.h:114:3: note: expanded from macro 'g_clear_pointer'
 > #  G_STMT_START {                                                               \
 > #  ^
 > /usr/include/glib-2.0/glib/gmacros.h:346:23: note: expanded from macro 'G_STMT_START'
 > ##define G_STMT_START  do
 > #                      ^
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:469:9: note: Returning; memory was released
 > #        vdagent_virtio_port_destroy(vportp);
 > #        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:269:9: note: Returning; memory was released
 > #        vdagent_virtio_port_do_write(vportp);
 > #        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:268:12: note: Left side of '&&' is true
 > #    while (*vportp && (*vportp)->write_buf)
 > #           ^
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:268:5: note: Loop condition is true.  Entering loop body
 > #    while (*vportp && (*vportp)->write_buf)
 > #    ^
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:269:9: note: Calling 'vdagent_virtio_port_do_write'
 > #        vdagent_virtio_port_do_write(vportp);
 > #        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:453:5: note: Taking false branch
 > #    if (!wbuf) {
 > #    ^
 > spice-vdagent-0.19.0/src/vdagentd/virtio-port.c:458:9: note: Use of memory after it is freed
 > #    if (wbuf->write_pos != wbuf->size) {
 > #        ^~~~~~~~~~~~~~~
 > #  456|       }
 > #  457|
 > #  458|->     if (wbuf->write_pos != wbuf->size) {
 > #  459|           syslog(LOG_ERR, "do_write: buffer is incomplete!!");
 > #  460|           return;

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>

Some code scanners (both Coverity and clang one) report that
prev_sel/prev_cond could be unreferenced while NULL.
Change condition to make clear a NULL pointer is not used.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>

Victor Toso authored 5 years ago and Frediano Ziglio committed 5 years ago

The g_list_length() function does iterate over both lists to compare
its length. Considering that we use this to check for true/false and
we will iterate again on both lists, we can do so once.

This also avoids covscan/clang warnings:

 | spice-vdagent-0.19.0/src/vdagent/device-info.c:216:27: warning: Access to field 'data' results in a dereference of a null pointer (loaded from variable 'lb')
 | #        PciDevice *devb = lb->data;
 | #                          ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:397:5: note: Taking false branch
 | #    if (!user_pci_addr) {
 | #    ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:407:22: note: Calling 'find_device_at_pci_address'
 | #    char *dev_path = find_device_at_pci_address(user_pci_addr, &vendor_id, &device_id);
 | #                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:329:5: note: Taking true branch
 | #    g_return_val_if_fail(pci_addr != NULL, NULL);
 | #    ^
 | /usr/include/glib-2.0/glib/gmessages.h:594:9: note: expanded from macro 'g_return_val_if_fail'
 | #     if G_LIKELY(expr) { } else                                         \
 | #        ^
 | /usr/include/glib-2.0/glib/gmacros.h:385:43: note: expanded from macro 'G_LIKELY'
 | ##define G_LIKELY(expr) (__builtin_expect (_G_BOOLEAN_EXPR((expr)), 1))
 | #                                          ^
 | /usr/include/glib-2.0/glib/gmacros.h:379:4: note: expanded from macro '_G_BOOLEAN_EXPR'
 | #   if (expr)                                    \
 | #   ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:329:5: note: Taking true branch
 | /usr/include/glib-2.0/glib/gmessages.h:594:6: note: expanded from macro 'g_return_val_if_fail'
 | #     if G_LIKELY(expr) { } else                                         \
 | #     ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:329:5: note: Loop condition is false.  Exiting loop
 | /usr/include/glib-2.0/glib/gmessages.h:593:40: note: expanded from macro 'g_return_val_if_fail'
 | ##define g_return_val_if_fail(expr,val)  G_STMT_START{                   \
 | #                                        ^
 | /usr/include/glib-2.0/glib/gmacros.h:346:23: note: expanded from macro 'G_STMT_START'
 | ##define G_STMT_START  do
 | #                      ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:330:5: note: Taking true branch
 | #    g_return_val_if_fail(device_id != NULL, NULL);
 | #    ^
 | /usr/include/glib-2.0/glib/gmessages.h:594:9: note: expanded from macro 'g_return_val_if_fail'
 | #     if G_LIKELY(expr) { } else                                         \
 | #        ^
 | /usr/include/glib-2.0/glib/gmacros.h:385:43: note: expanded from macro 'G_LIKELY'
 | ##define G_LIKELY(expr) (__builtin_expect (_G_BOOLEAN_EXPR((expr)), 1))
 | #                                          ^
 | /usr/include/glib-2.0/glib/gmacros.h:379:4: note: expanded from macro '_G_BOOLEAN_EXPR'
 | #   if (expr)                                    \
 | #   ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:330:5: note: Taking true branch
 | /usr/include/glib-2.0/glib/gmessages.h:594:6: note: expanded from macro 'g_return_val_if_fail'
 | #     if G_LIKELY(expr) { } else                                         \
 | #     ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:330:5: note: Loop condition is false.  Exiting loop
 | /usr/include/glib-2.0/glib/gmessages.h:593:40: note: expanded from macro 'g_return_val_if_fail'
 | ##define g_return_val_if_fail(expr,val)  G_STMT_START{                   \
 | #                                        ^
 | /usr/include/glib-2.0/glib/gmacros.h:346:23: note: expanded from macro 'G_STMT_START'
 | ##define G_STMT_START  do
 | #                      ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:331:5: note: Taking true branch
 | #    g_return_val_if_fail(vendor_id != NULL, NULL);
 | #    ^
 | /usr/include/glib-2.0/glib/gmessages.h:594:9: note: expanded from macro 'g_return_val_if_fail'
 | #     if G_LIKELY(expr) { } else                                         \
 | #        ^
 | /usr/include/glib-2.0/glib/gmacros.h:385:43: note: expanded from macro 'G_LIKELY'
 | ##define G_LIKELY(expr) (__builtin_expect (_G_BOOLEAN_EXPR((expr)), 1))
 | #                                          ^
 | /usr/include/glib-2.0/glib/gmacros.h:379:4: note: expanded from macro '_G_BOOLEAN_EXPR'
 | #   if (expr)                                    \
 | #   ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:331:5: note: Taking true branch
 | /usr/include/glib-2.0/glib/gmessages.h:594:6: note: expanded from macro 'g_return_val_if_fail'
 | #     if G_LIKELY(expr) { } else                                         \
 | #     ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:331:5: note: Loop condition is false.  Exiting loop
 | /usr/include/glib-2.0/glib/gmessages.h:593:40: note: expanded from macro 'g_return_val_if_fail'
 | ##define g_return_val_if_fail(expr,val)  G_STMT_START{                   \
 | #                                        ^
 | /usr/include/glib-2.0/glib/gmacros.h:346:23: note: expanded from macro 'G_STMT_START'
 | ##define G_STMT_START  do
 | #                      ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:334:5: note: Loop condition is true.  Entering loop body
 | #    for (int i = 0; i < 10; ++i) {
 | #    ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:340:13: note: Assuming the condition is false
 | #        if (stat(dev_path, &buf) != 0) {
 | #            ^~~~~~~~~~~~~~~~~~~~~~~~~
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:340:9: note: Taking false branch
 | #        if (stat(dev_path, &buf) != 0) {
 | #        ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:355:13: note: Assuming the condition is false
 | #        if (realpath(sys_path, device_link) == NULL) {
 | #            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:355:9: note: Taking false branch
 | #        if (realpath(sys_path, device_link) == NULL) {
 | #        ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:361:36: note: Calling 'parse_pci_address_from_sysfs_path'
 | #        PciAddress *drm_pci_addr = parse_pci_address_from_sysfs_path(device_link);
 | #                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:129:9: note: Assuming 'pos' is non-null
 | #    if (!pos) {
 | #        ^~~~
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:129:5: note: Taking false branch
 | #    if (!pos) {
 | #    ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:136:9: note: Assuming 'pos' is non-null
 | #    if (!pos) {
 | #        ^~~~
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:136:5: note: Taking false branch
 | #    if (!pos) {
 | #    ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:142:9: note: Assuming 'pos' is non-null
 | #    if (!pos) {
 | #        ^~~~
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:142:5: note: Taking false branch
 | #    if (!pos) {
 | #    ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:146:27: note: Calling 'pci_address_new'
 | #    PciAddress *address = pci_address_new();
 | #                          ^~~~~~~~~~~~~~~~~
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:61:12: note: Taking false branch
 | #    return g_new0(PciAddress, 1);
 | #           ^
 | /usr/include/glib-2.0/glib/gmem.h:279:42: note: expanded from macro 'g_new0'
 | ##define g_new0(struct_type, n_structs)                  _G_NEW (struct_type, n_structs, malloc0)
 | #                                                        ^
 | /usr/include/glib-2.0/glib/gmem.h:211:4: note: expanded from macro '_G_NEW'
 | #          if (__s == 1)                                         \
 | #          ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:61:12: note: Left side of '&&' is false
 | /usr/include/glib-2.0/glib/gmem.h:279:42: note: expanded from macro 'g_new0'
 | ##define g_new0(struct_type, n_structs)                  _G_NEW (struct_type, n_structs, malloc0)
 | #                                                        ^
 | /usr/include/glib-2.0/glib/gmem.h:213:40: note: expanded from macro '_G_NEW'
 | #          else if (__builtin_constant_p (__n) &&                \
 | #                                              ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:61:12: note: Null pointer value stored to field 'devices'
 | #    return g_new0(PciAddress, 1);
 | #           ^~~~~~~~~~~~~~~~~~~~~
 | /usr/include/glib-2.0/glib/gmem.h:279:42: note: expanded from macro 'g_new0'
 | ##define g_new0(struct_type, n_structs)                  _G_NEW (struct_type, n_structs, malloc0)
 | #                                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 | /usr/include/glib-2.0/glib/gmem.h:217:12: note: expanded from macro '_G_NEW'
 | #            __p = g_##func##_n (__n, __s);                      \
 | #                  ^~~~~~~~~~~~~~~~~~~~~~~
 | <scratch space>:76:1: note: expanded from here
 | #g_malloc0_n
 | #^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:146:27: note: Returning from 'pci_address_new'
 | #    PciAddress *address = pci_address_new();
 | #                          ^~~~~~~~~~~~~~~~~
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:149:5: note: Loop condition is true.  Entering loop body
 | #    while (pos) {
 | #    ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:150:26: note: Taking false branch
 | #        PciDevice *dev = g_new0(PciDevice, 1);
 | #                         ^
 | /usr/include/glib-2.0/glib/gmem.h:279:42: note: expanded from macro 'g_new0'
 | ##define g_new0(struct_type, n_structs)                  _G_NEW (struct_type, n_structs, malloc0)
 | #                                                        ^
 | /usr/include/glib-2.0/glib/gmem.h:211:4: note: expanded from macro '_G_NEW'
 | #          if (__s == 1)                                         \
 | #          ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:150:26: note: Left side of '&&' is false
 | /usr/include/glib-2.0/glib/gmem.h:279:42: note: expanded from macro 'g_new0'
 | ##define g_new0(struct_type, n_structs)                  _G_NEW (struct_type, n_structs, malloc0)
 | #                                                        ^
 | /usr/include/glib-2.0/glib/gmem.h:213:40: note: expanded from macro '_G_NEW'
 | #          else if (__builtin_constant_p (__n) &&                \
 | #                                              ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:152:9: note: Taking true branch
 | #        if (!parse_pci_device(pos + 1, next, dev)) {
 | #        ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:154:13: note:  Execution continues on line 159
 | #            break;
 | #            ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:361:36: note: Returning from 'parse_pci_address_from_sysfs_path'
 | #        PciAddress *drm_pci_addr = parse_pci_address_from_sysfs_path(device_link);
 | #                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:362:9: note: Taking false branch
 | #        if (!drm_pci_addr) {
 | #        ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:367:14: note: Calling 'compare_addresses'
 | #        if (!compare_addresses(pci_addr, drm_pci_addr)) {
 | #             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:207:11: note: Assuming the condition is true
 | #    if (!(a->domain == b->domain
 | #          ^~~~~~~~~~~~~~~~~~~~~~
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:207:11: note: Left side of '&&' is true
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:208:12: note: Assuming the condition is true
 | #        && g_list_length(a->devices) == g_list_length(b->devices))) {
 | #           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:207:5: note: Taking false branch
 | #    if (!(a->domain == b->domain
 | #    ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:212:35: note: 'lb' initialized to a null pointer value
 | #    for (GList *la = a->devices, *lb = b->devices;
 | #                                  ^~
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:213:10: note: Assuming 'la' is not equal to NULL
 | #         la != NULL;
 | #         ^~~~~~~~~~
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:212:5: note: Loop condition is true.  Entering loop body
 | #    for (GList *la = a->devices, *lb = b->devices;
 | #    ^
 | spice-vdagent-0.19.0/src/vdagent/device-info.c:216:27: note: Access to field 'data' results in a dereference of a null pointer (loaded from variable 'lb')
 | #        PciDevice *devb = lb->data;
 | #                          ^~
 | #  214|            la = la->next, lb = lb->next) {
 | #  215|           PciDevice *deva = la->data;
 | #  216|->         PciDevice *devb = lb->data;
 | #  217|
 | #  218|           if (deva->slot != devb->slot

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>

No reasons to have it mutable.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>

The structure is not that small and is not necessary to copy
the value.
This also removed a Coverity warning.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>

The structure is not that small and is not necessary to copy
the value.
This also removed a Coverity warning.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>

Victor Toso authored 5 years ago and Frediano Ziglio committed 5 years ago

Seems to be a false positive but as this message only happens when user
client connects, we can copy this array to make compiling warn free.

 > src/vdagent/vdagent.c: In function ‘daemon_read_complete’:
 > src/vdagent/vdagent.c:226:71: error: taking address of packed member of
 >     ‘struct VDAgentAudioVolumeSync’ may result in an unaligned pointer
 >     value [-Werror=address-of-packed-member]
 >   226 |             vdagent_audio_playback_sync(avs->mute, avs->nchannels, avs->volume);
 >       |                                                                    ~~~^~~~~~~~
 > src/vdagent/vdagent.c:228:69: error: taking address of packed member of
 >     ‘struct VDAgentAudioVolumeSync’ may result in an unaligned pointer
 >     value [-Werror=address-of-packed-member]
 >   228 |             vdagent_audio_record_sync(avs->mute, avs->nchannels, avs->volume);
 >       |                                                                  ~~~^~~~~~~~

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>

Victor Toso authored 5 years ago and Frediano Ziglio committed 5 years ago

The struct type for width/height is uint32_t while we are trying to
access and change it with int* - code can be improved a bit in following
patches but this one fixes the warning by copying the value from the
struct and copying back new value afterwards.

Also:
- Moved variables to internal scope;
- Added braces to inner if;

 > src/vdagent/x11-randr.c: In function ‘zero_base_monitors’:
 >     src/vdagent/x11-randr.c:621:28: error: taking address of packed member of
 >     ‘struct VDAgentMonConfig’ may result in an unaligned pointer value
 > [-Werror=address-of-packed-member]
 >   621 |         mon_width = (int *)&mon_config->monitors[i].width;
 >       |                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 > src/vdagent/x11-randr.c:622:29: error: taking address of packed member of
 >     ‘struct VDAgentMonConfig’ may result in an unaligned pointer value
 >     [-Werror=address-of-packed-member]
 >   622 |         mon_height = (int *)&mon_config->monitors[i].height;
 >       |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>

Victor Toso authored 5 years ago and Frediano Ziglio committed 5 years ago

Since 28618682 "randr: remove monitors.xml on auto-configuration" in
2015-04-10 by Marc-Andre Lureau <marcandre.lureau@redhat.com> this
file includes glib.h and can remove those helpers.

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>

Victor Toso authored 5 years ago and Frediano Ziglio committed 5 years ago

Found by covscan:

 | spice-vdagent-0.19.0/src/vdagent/vdagent.c:432:9: warning: Potential leak of memory pointed to by 'orig_argv'
 | #        g_printerr("Invalid arguments, %s\n", error->message);
 | #        ^
 | spice-vdagent-0.19.0/src/vdagent/vdagent.c:418:24: note: Memory is allocated
 | #    char **orig_argv = g_memdup(argv, sizeof(char*) * (argc+1));
 | #                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 | spice-vdagent-0.19.0/src/vdagent/vdagent.c:431:9: note: Assuming 'error' is not equal to NULL
 | #    if (error != NULL) {
 | #        ^~~~~~~~~~~~~
 | spice-vdagent-0.19.0/src/vdagent/vdagent.c:431:5: note: Taking true branch
 | #    if (error != NULL) {
 | #    ^
 | spice-vdagent-0.19.0/src/vdagent/vdagent.c:432:9: note: Potential leak of memory pointed to by 'orig_argv'
 | #        g_printerr("Invalid arguments, %s\n", error->message);
 | #        ^
 | #  430|
 | #  431|       if (error != NULL) {
 | #  432|->         g_printerr("Invalid arguments, %s\n", error->message);
 | #  433|           g_clear_error(&error);
 | #  434|           return -1;

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>

Missed this when adding 'md' prefix to README with
0193a537 in 2019-03-08 by Victor Toso <me@victortoso.com>

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>

The requirement to the unreleased spice-protocol was added with
56c2d297 in 2019-01-28 by Lukáš Hrázký <lhrazky@redhat.com> but
next stable release of spice-protocol is 0.14.0

The configure check is correct but seems better to me to check against
a valid release instead of checking with a non existing one.

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>