Thu Jun 18 18:58:36 UTC 2020

Rebased ref, commits from common ancestor:
commit 73cd48a176789b3e0e86497ac20d82c0c26b53f5
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Thu Jun 4 06:43:03 2020 +0100

    Enable -Wshadow warning
    
    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 at gmail.com>

diff --git a/m4/spice-compile-warnings.m4 b/m4/spice-compile-warnings.m4
index 6fd00db9..d87203d7 100644
--- a/m4/spice-compile-warnings.m4
+++ b/m4/spice-compile-warnings.m4
@@ -168,6 +168,9 @@ AC_DEFUN([SPICE_COMPILE_WARNINGS],[
 
     gl_WARN_ADD([-Wno-missing-field-initializers])
 
+    # -Wshadow detects shadowing of arguments, globals and C++ attributes
+    gl_WARN_ADD([-Wshadow])
+
     WARN_CXXFLAGS="$WARN_CFLAGS"
     AC_SUBST([WARN_CXXFLAGS])
 
diff --git a/meson.build b/meson.build
index b2fb018b..df4caa58 100644
--- a/meson.build
+++ b/meson.build
@@ -197,6 +197,7 @@ spice_server_global_cxxflags += [
   '-Wno-narrowing',
   '-Wno-missing-field-initializers',
   '-Wno-deprecated-declarations',
+  '-Wshadow',
 ]
 add_project_arguments(cxx_compiler.get_supported_arguments(spice_server_global_cxxflags),
                       language : 'cpp')
diff --git a/server/dispatcher.cpp b/server/dispatcher.cpp
index 78726b2e..5abd2e89 100644
--- a/server/dispatcher.cpp
+++ b/server/dispatcher.cpp
@@ -45,8 +45,8 @@ struct DispatcherMessage {
 
 struct DispatcherPrivate {
     SPICE_CXX_GLIB_ALLOCATOR
-    DispatcherPrivate(uint32_t max_message_type):
-        max_message_type(max_message_type)
+    DispatcherPrivate(uint32_t _max_message_type):
+        max_message_type(_max_message_type)
     {
     }
     ~DispatcherPrivate();
@@ -238,7 +238,7 @@ void DispatcherPrivate::handle_event(int fd, int event, DispatcherPrivate* priv)
     }
 }
 
-void DispatcherPrivate::send_message(const DispatcherMessage& msg, void *payload)
+void DispatcherPrivate::send_message(const DispatcherMessage& msg, void *msg_payload)
 {
     uint32_t ack;
 
@@ -248,7 +248,7 @@ void DispatcherPrivate::send_message(const DispatcherMessage& msg, void *payload
                   msg.type);
         goto unlock;
     }
-    if (write_safe(send_fd, (uint8_t*) payload, msg.size) == -1) {
+    if (write_safe(send_fd, (uint8_t*) msg_payload, msg.size) == -1) {
         g_warning("error: failed to send message body for message %d",
                   msg.type);
         goto unlock;
diff --git a/server/display-channel.cpp b/server/display-channel.cpp
index 398aed33..7449e8bf 100644
--- a/server/display-channel.cpp
+++ b/server/display-channel.cpp
@@ -324,9 +324,8 @@ static void pipes_add_drawable_after(DisplayChannel *display,
     RedDrawablePipeItem *dpi_pos_after;
     DisplayChannelClient *dcc;
     int num_other_linked = 0;
-    GList *l;
 
-    for (l = pos_after->pipes; l != NULL; l = l->next) {
+    for (GList *l = pos_after->pipes; l != NULL; l = l->next) {
         dpi_pos_after = (RedDrawablePipeItem*) l->data;
 
         num_other_linked++;
@@ -341,8 +340,7 @@ static void pipes_add_drawable_after(DisplayChannel *display,
         spice_debug("TODO: not O(n^2)");
         FOREACH_DCC(display, dcc) {
             int sent = 0;
-            GList *l;
-            for (l = pos_after->pipes; l != NULL; l = l->next) {
+            for (GList *l = pos_after->pipes; l != NULL; l = l->next) {
                 dpi_pos_after = (RedDrawablePipeItem*) l->data;
                 if (dpi_pos_after->dcc == dcc) {
                     sent = 1;
@@ -762,9 +760,9 @@ static void exclude_region(DisplayChannel *display, Ring *ring, RingItem *ring_i
             } else if (now->type == TREE_ITEM_TYPE_CONTAINER) {
                 /* if this sibling is a container type, descend into the
                  * container's child ring and continue iterating */
-                Container *container = CONTAINER(now);
-                if ((ring_item = ring_get_head(&container->items))) {
-                    ring = &container->items;
+                Container *now_container = CONTAINER(now);
+                if ((ring_item = ring_get_head(&now_container->items))) {
+                    ring = &now_container->items;
                     spice_assert(SPICE_CONTAINEROF(ring_item, TreeItem, siblings_link)->container);
                     continue;
                 }
diff --git a/server/inputs-channel.cpp b/server/inputs-channel.cpp
index f744e2a3..d712220f 100644
--- a/server/inputs-channel.cpp
+++ b/server/inputs-channel.cpp
@@ -459,7 +459,7 @@ void InputsChannelClient::migrate()
     RedChannelClient::migrate();
 }
 
-void InputsChannel::push_keyboard_modifiers(uint8_t modifiers)
+void InputsChannel::push_keyboard_modifiers()
 {
     if (!is_connected() || src_during_migrate) {
         return;
@@ -472,14 +472,14 @@ SPICE_GNUC_VISIBLE int spice_server_kbd_leds(SpiceKbdInstance *sin, int leds)
     InputsChannel *inputs_channel = sin->st->inputs;
     if (inputs_channel) {
         inputs_channel->modifiers = leds;
-        inputs_channel->push_keyboard_modifiers(leds);
+        inputs_channel->push_keyboard_modifiers();
     }
     return 0;
 }
 
 void InputsChannel::key_modifiers_sender(InputsChannel *inputs)
 {
-    inputs->push_keyboard_modifiers(inputs->modifiers);
+    inputs->push_keyboard_modifiers();
 }
 
 void InputsChannelClient::handle_migrate_flush_mark()
@@ -575,13 +575,13 @@ bool InputsChannel::has_tablet() const
     return tablet != NULL;
 }
 
-void InputsChannel::detach_tablet(SpiceTabletInstance *tablet)
+void InputsChannel::detach_tablet(SpiceTabletInstance *old_tablet)
 {
-    if (tablet != NULL && tablet == this->tablet) {
-        spice_tablet_state_free(tablet->st);
-        tablet->st = NULL;
+    if (old_tablet != NULL && old_tablet == tablet) {
+        spice_tablet_state_free(old_tablet->st);
+        old_tablet->st = NULL;
     }
-    this->tablet = NULL;
+    tablet = NULL;
 }
 
 bool InputsChannel::is_src_during_migrate() const
diff --git a/server/inputs-channel.h b/server/inputs-channel.h
index bbbfcd12..3f78f304 100644
--- a/server/inputs-channel.h
+++ b/server/inputs-channel.h
@@ -70,7 +70,7 @@ private:
     void release_keys();
     void sync_locks(uint8_t scan);
     void activate_modifiers_watch();
-    void push_keyboard_modifiers(uint8_t modifiers);
+    void push_keyboard_modifiers();
     static void key_modifiers_sender(InputsChannel *inputs);
 };
 
diff --git a/server/main-channel.cpp b/server/main-channel.cpp
index 8dfe5631..388d0e46 100644
--- a/server/main-channel.cpp
+++ b/server/main-channel.cpp
@@ -130,9 +130,9 @@ MainChannel::registered_new_channel(RedChannel *channel)
     pipes_add(registered_channel_item_new(channel));
 }
 
-void MainChannel::migrate_switch(RedsMigSpice *mig_target)
+void MainChannel::migrate_switch(RedsMigSpice *new_mig_target)
 {
-    main_channel_fill_mig_target(this, mig_target);
+    main_channel_fill_mig_target(this, new_mig_target);
     pipes_add_type(RED_PIPE_ITEM_TYPE_MAIN_MIGRATE_SWITCH_HOST);
 }
 
@@ -249,9 +249,9 @@ static int main_channel_connect_seamless(MainChannel *main_channel)
     return main_channel->num_clients_mig_wait;
 }
 
-int MainChannel::migrate_connect(RedsMigSpice *mig_target, int try_seamless)
+int MainChannel::migrate_connect(RedsMigSpice *new_mig_target, int try_seamless)
 {
-    main_channel_fill_mig_target(this, mig_target);
+    main_channel_fill_mig_target(this, new_mig_target);
     num_clients_mig_wait = 0;
 
     if (!is_connected()) {
diff --git a/server/main-dispatcher.cpp b/server/main-dispatcher.cpp
index 73cad07a..aeceb0b5 100644
--- a/server/main-dispatcher.cpp
+++ b/server/main-dispatcher.cpp
@@ -173,9 +173,9 @@ void MainDispatcher::client_disconnect(RedClient *client)
  * Reds routines shouldn't be exposed. Instead reds.cpp should register the callbacks,
  * and the corresponding operations should be made only via main_dispatcher.
  */
-MainDispatcher::MainDispatcher(RedsState *reds):
+MainDispatcher::MainDispatcher(RedsState *_reds):
     Dispatcher(MAIN_DISPATCHER_NUM_MESSAGES),
-    reds(reds),
+    reds(_reds),
     thread_id(pthread_self())
 {
     set_opaque(reds);
diff --git a/server/red-channel-client.cpp b/server/red-channel-client.cpp
index 8e0e7fb5..09174144 100644
--- a/server/red-channel-client.cpp
+++ b/server/red-channel-client.cpp
@@ -291,13 +291,13 @@ void RedChannelClientPrivate::restart_ping_timer()
     start_ping_timer(timeout);
 }
 
-RedChannelClientPrivate::RedChannelClientPrivate(RedChannel *channel,
-                                                 RedClient *client,
-                                                 RedStream *stream,
+RedChannelClientPrivate::RedChannelClientPrivate(RedChannel *_channel,
+                                                 RedClient *_client,
+                                                 RedStream *_stream,
                                                  RedChannelCapabilities *caps,
-                                                 bool monitor_latency):
-    channel(channel),
-    client(client), stream(stream)
+                                                 bool _monitor_latency):
+    channel(_channel),
+    client(_client), stream(_stream)
 {
     // blocks send message (maybe use send_data.blocked + block flags)
     ack_data.messages_window = ~0;
diff --git a/server/red-channel.cpp b/server/red-channel.cpp
index 5d60096d..a6d463be 100644
--- a/server/red-channel.cpp
+++ b/server/red-channel.cpp
@@ -65,15 +65,16 @@ struct RedChannelPrivate
 {
     SPICE_CXX_GLIB_ALLOCATOR
 
-    RedChannelPrivate(RedsState *reds, uint32_t type, uint32_t id, RedChannel::CreationFlags flags,
-                      SpiceCoreInterfaceInternal *core, Dispatcher *dispatcher):
-        type(type), id(id),
-        core(core ? core : reds_get_core_interface(reds)),
+    RedChannelPrivate(RedsState *_reds, uint32_t _type, uint32_t _id,
+                      RedChannel::CreationFlags flags,
+                      SpiceCoreInterfaceInternal *_core, Dispatcher *_dispatcher):
+        type(_type), id(_id),
+        core(_core ? _core : reds_get_core_interface(_reds)),
         handle_acks(!!(flags & RedChannel::HandleAcks)),
-        parser(spice_get_client_channel_parser(type, nullptr)),
+        parser(spice_get_client_channel_parser(_type, nullptr)),
         migration_flags(flags & RedChannel::MigrateAll),
-        dispatcher(dispatcher),
-        reds(reds)
+        dispatcher(_dispatcher),
+        reds(_reds)
     {
         thread_id = pthread_self();
     }
diff --git a/server/red-client.cpp b/server/red-client.cpp
index 04ce7113..d5cf1c30 100644
--- a/server/red-client.cpp
+++ b/server/red-client.cpp
@@ -30,8 +30,8 @@ RedClient::~RedClient()
     pthread_mutex_destroy(&lock);
 }
 
-RedClient::RedClient(RedsState *reds, bool migrated):
-    reds(reds),
+RedClient::RedClient(RedsState *_reds, bool migrated):
+    reds(_reds),
     during_target_migrate(migrated)
 {
     pthread_mutex_init(&lock, NULL);
diff --git a/server/red-replay-qxl.cpp b/server/red-replay-qxl.cpp
index be8ab7d0..13431094 100644
--- a/server/red-replay-qxl.cpp
+++ b/server/red-replay-qxl.cpp
@@ -478,9 +478,9 @@ static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
         if (qxl_flags & QXL_BITMAP_DIRECT) {
             qxl->bitmap.data = QXLPHYSICAL_FROM_PTR(red_replay_image_data_flat(replay, &bitmap_size));
         } else {
-            uint8_t *data = NULL;
-            size = red_replay_data_chunks(replay, "bitmap.data", &data, 0);
-            qxl->bitmap.data = QXLPHYSICAL_FROM_PTR(data);
+            uint8_t *bitmap_data = NULL;
+            size = red_replay_data_chunks(replay, "bitmap.data", &bitmap_data, 0);
+            qxl->bitmap.data = QXLPHYSICAL_FROM_PTR(bitmap_data);
             if (size != bitmap_size) {
                 g_warning("bad image, %" G_GSIZE_FORMAT " != %" G_GSIZE_FORMAT, size, bitmap_size);
                 return NULL;
diff --git a/server/red-stream-device.cpp b/server/red-stream-device.cpp
index 73dd6527..9fdcd319 100644
--- a/server/red-stream-device.cpp
+++ b/server/red-stream-device.cpp
@@ -189,10 +189,10 @@ StreamDevice::handle_msg_invalid(SpiceCharDeviceInstance *sin, const char *error
         write_buffer_get_server(total_size, false);
     buf->buf_used = total_size;
 
-    StreamDevHeader *const hdr = (StreamDevHeader *)buf->buf;
-    fill_dev_hdr(hdr, STREAM_TYPE_NOTIFY_ERROR, msg_size);
+    StreamDevHeader *const header = (StreamDevHeader *)buf->buf;
+    fill_dev_hdr(header, STREAM_TYPE_NOTIFY_ERROR, msg_size);
 
-    StreamMsgNotifyError *const error = (StreamMsgNotifyError *)(hdr+1);
+    StreamMsgNotifyError *const error = (StreamMsgNotifyError *)(header+1);
     error->error_code = GUINT32_TO_LE(0);
     strcpy((char *) error->msg, error_msg);
 
diff --git a/server/safe-list.hpp b/server/safe-list.hpp
index 1bcf27a1..3b9bd5d9 100644
--- a/server/safe-list.hpp
+++ b/server/safe-list.hpp
@@ -110,9 +110,9 @@ class safe_list<T>::iterator: public std::iterator<std::forward_iterator_tag, T>
     typedef typename std::forward_list<T,Mallocator<T>>::iterator wrapped;
     wrapped curr, next;
 public:
-    iterator(wrapped curr) :
-        curr(curr),
-        next(curr != wrapped() ? ++curr : wrapped())
+    iterator(wrapped _curr) :
+        curr(_curr),
+        next(_curr != wrapped() ? ++_curr : wrapped())
     {
     }
     iterator& operator++()
diff --git a/server/spicevmc.cpp b/server/spicevmc.cpp
index 265f6049..e859ba8f 100644
--- a/server/spicevmc.cpp
+++ b/server/spicevmc.cpp
@@ -43,6 +43,7 @@
 #define QUEUED_DATA_LIMIT (1024*1024)
 
 struct RedVmcChannel;
+class VmcChannelClient;
 
 typedef struct RedVmcPipeItem {
     RedPipeItem base;
@@ -77,7 +78,7 @@ struct RedVmcChannel: public RedChannel
     void on_connect(RedClient *client, RedStream *stream, int migration,
                     RedChannelCapabilities *caps) override;
 
-    RedChannelClient *rcc;
+    VmcChannelClient *rcc;
     RedCharDevice *chardev; /* weak */
     SpiceCharDeviceInstance *chardev_sin;
     RedVmcPipeItem *pipe_item;
@@ -623,21 +624,20 @@ void RedVmcChannel::on_connect(RedClient *client, RedStream *stream, int migrati
     vmc_channel = this;
     sin = vmc_channel->chardev_sin;
 
-    if (vmc_channel->rcc) {
+    if (rcc) {
         red_channel_warning(this,
                             "channel client (%p) already connected, refusing second connection",
-                            vmc_channel->rcc);
+                            rcc);
         // TODO: notify client in advance about the in use channel using
         // SPICE_MSG_MAIN_CHANNEL_IN_USE (for example)
         red_stream_free(stream);
         return;
     }
 
-    auto rcc = vmc_channel_client_create(this, client, stream, caps);
+    rcc = vmc_channel_client_create(this, client, stream, caps);
     if (!rcc) {
         return;
     }
-    vmc_channel->rcc = rcc;
     vmc_channel->queued_data = 0;
     rcc->ack_zero_messages_window();
 
@@ -689,9 +689,9 @@ void RedCharDeviceSpiceVmc::port_event(uint8_t event)
 }
 
 RedCharDeviceSpiceVmc::RedCharDeviceSpiceVmc(SpiceCharDeviceInstance *sin, RedsState *reds,
-                                             RedVmcChannel *channel):
+                                             RedVmcChannel *_channel):
     RedCharDevice(reds, sin, 0, 128),
-    channel(channel)
+    channel(_channel)
 {
     if (channel) {
         channel->chardev = this;
diff --git a/server/tests/test-display-base.cpp b/server/tests/test-display-base.cpp
index c30d41fb..969ef687 100644
--- a/server/tests/test-display-base.cpp
+++ b/server/tests/test-display-base.cpp
@@ -240,7 +240,8 @@ test_spice_create_update_from_bitmap(uint32_t surface_id,
     return update;
 }
 
-static SimpleSpiceUpdate *test_spice_create_update_solid(uint32_t surface_id, QXLRect bbox, uint32_t color)
+static SimpleSpiceUpdate *test_spice_create_update_solid(uint32_t surface_id, QXLRect bbox,
+                                                         uint32_t solid_color)
 {
     uint8_t *bitmap;
     uint32_t *dst;
@@ -255,7 +256,7 @@ static SimpleSpiceUpdate *test_spice_create_update_solid(uint32_t surface_id, QX
     dst = SPICE_ALIGNED_CAST(uint32_t *, bitmap);
 
     for (i = 0 ; i < bh * bw ; ++i, ++dst) {
-        *dst = color;
+        *dst = solid_color;
     }
 
     return test_spice_create_update_from_bitmap(surface_id, bbox, bitmap, 0, NULL);
@@ -882,9 +883,9 @@ void test_set_simple_command_list(Test *test, const int *simple_commands, int nu
     }
 }
 
-void test_set_command_list(Test *test, Command *commands, int num_commands)
+void test_set_command_list(Test *test, Command *new_commands, int num_commands)
 {
-    test->commands = commands;
+    test->commands = new_commands;
     test->num_commands = num_commands;
 }
 
diff --git a/server/tests/test-smartcard.cpp b/server/tests/test-smartcard.cpp
index f3eced41..8c127e2e 100644
--- a/server/tests/test-smartcard.cpp
+++ b/server/tests/test-smartcard.cpp
@@ -131,15 +131,15 @@ static void send_data(int socket, uint32_t type, uint32_t reader_id)
     g_assert_cmpint(socket_write(socket, &msg.type, sizeof(msg)-4), ==, sizeof(msg)-4);
 }
 
-static void check_data(VmcEmu *vmc)
+static void check_data(VmcEmu *vmc_emu)
 {
     g_assert_cmpint(device_expected.offset, !=, 0);
-    if (vmc->write_pos < device_expected.offset) {
+    if (vmc_emu->write_pos < device_expected.offset) {
         return;
     }
-    g_assert_cmpint(vmc->write_pos, ==, device_expected.offset);
-    g_assert_true(memcmp(vmc->write_buf, device_expected.buffer, device_expected.offset) == 0);
-    vmc->write_pos = 0;
+    g_assert_cmpint(vmc_emu->write_pos, ==, device_expected.offset);
+    g_assert_true(memcmp(vmc_emu->write_buf, device_expected.buffer, device_expected.offset) == 0);
+    vmc_emu->write_pos = 0;
 
     next_test();
 }
diff --git a/server/utils.hpp b/server/utils.hpp
index eac4191a..4ba9de80 100644
--- a/server/utils.hpp
+++ b/server/utils.hpp
@@ -51,7 +51,7 @@ public:
     unique_link(): p(new T())
     {
     }
-    unique_link(T* p): p(p)
+    unique_link(T* ptr): p(ptr)
     {
     }
     ~unique_link()
@@ -124,14 +124,14 @@ class shared_ptr
 {
 friend class weak_ptr<T>;
 public:
-    explicit shared_ptr(T *p=nullptr): p(p)
+    explicit shared_ptr(T *ptr=nullptr): p(ptr)
     {
         if (p) {
             shared_ptr_add_ref(p);
         }
     }
     template <class Q>
-    explicit shared_ptr(Q *p): shared_ptr(static_cast<T*>(p))
+    explicit shared_ptr(Q *ptr): shared_ptr(static_cast<T*>(ptr))
     {
     }
     shared_ptr(const shared_ptr& rhs): p(rhs.p)
@@ -214,7 +214,7 @@ public:
 private:
     T* p;
     // for weak_ptr
-    explicit shared_ptr(T *p, bool dummy): p(p)
+    explicit shared_ptr(T *ptr, bool dummy): p(ptr)
     {
     }
 };
@@ -286,7 +286,7 @@ template <typename T>
 class weak_ptr
 {
 public:
-    explicit weak_ptr(T *p=nullptr): p(p)
+    explicit weak_ptr(T *ptr=nullptr): p(ptr)
     {
         if (p) {
             weak_ptr_add_ref(p);
commit 8875e79c7cccb1080707fea01f784ff6e0fe471e
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Thu Jun 18 12:49:15 2020 +0100

    reds: Use socket_close instead of close
    
    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 at gmail.com>
    Acked-by: Uri Lublin <ulublin at redhat.com>

diff --git a/server/reds.cpp b/server/reds.cpp
index 10841b68..1abb9fe9 100644
--- a/server/reds.cpp
+++ b/server/reds.cpp
@@ -2470,8 +2470,7 @@ static void reds_accept_ssl_connection(int fd, int event, void *data)
     }
 
     if (!(link = reds_init_client_ssl_connection(reds, socket))) {
-        close(socket);
-        return;
+        socket_close(socket);
     }
 }
 
@@ -2486,8 +2485,9 @@ static void reds_accept(int fd, int event, void *data)
         return;
     }
 
-    if (spice_server_add_client(reds, socket, 0) < 0)
-        close(socket);
+    if (spice_server_add_client(reds, socket, 0) < 0) {
+        socket_close(socket);
+    }
 }
 
 
commit 21323c8b6f09d8f633910cc990e13e095c062d95
Author: Uri Lublin <uril at redhat.com>
Date:   Sun May 17 18:08:44 2020 +0300

    glz_dictionary_window_add_image: error out when failed to alloc an image
    
    __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 at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/glz-encoder-dict.c b/server/glz-encoder-dict.c
index 494d8878..72e3d1bb 100644
--- a/server/glz-encoder-dict.c
+++ b/server/glz-encoder-dict.c
@@ -495,6 +495,9 @@ static WindowImage *glz_dictionary_window_add_image(SharedDictionary *dict, LzIm
     uint8_t* lines = first_lines;
     // alloc image info,update used head tail,  if used_head null - update  head
     WindowImage *image = __glz_dictionary_window_alloc_image(dict);
+    if (!image) {
+        dict->cur_usr->error(dict->cur_usr, "glz-dictionary failed to allocate an image\n");
+    }
     image->id = dict->last_image_id++;
     image->size = image_size;
     image->type = image_type;
commit 031ab732dded42f10dc15fe7035b1f41158cf7ee
Author: Uri Lublin <uril at redhat.com>
Date:   Sun May 17 17:55:02 2020 +0300

    glz_enc_dictionary_restore: return NULL upon failure
    
    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 at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/glz-encoder-dict.c b/server/glz-encoder-dict.c
index 7bf16bfb..494d8878 100644
--- a/server/glz-encoder-dict.c
+++ b/server/glz-encoder-dict.c
@@ -197,6 +197,9 @@ GlzEncDictContext *glz_enc_dictionary_restore(GlzEncDictRestoreData *restore_dat
     }
     SharedDictionary *ret = (SharedDictionary *)glz_enc_dictionary_create(
             restore_data->size, restore_data->max_encoders, usr);
+    if (!ret) {
+        return NULL;
+    }
     ret->last_image_id = restore_data->last_image_id;
     return ((GlzEncDictContext *)ret);
 }
commit 954eabaeb76a0f93a32210b6bf63157ad2c0fb22
Author: Uri Lublin <uril at redhat.com>
Date:   Wed Jun 17 11:52:05 2020 +0300

    test-websocket: check setsockopt return value
    
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/tests/test-websocket.c b/server/tests/test-websocket.c
index 2115411e..701f5408 100644
--- a/server/tests/test-websocket.c
+++ b/server/tests/test-websocket.c
@@ -146,7 +146,10 @@ main(int argc, char **argv)
     }
 
     int enable = 1;
-    setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &enable, sizeof(enable));
+    if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR,
+                   (const void *) &enable, sizeof(enable)) < 0) {
+        err(1, "setsockopt reuseaddr");
+    }
 
     if (non_blocking) {
         red_socket_set_non_blocking(sock, true);
@@ -200,7 +203,10 @@ handle_client(int new_sock)
     }
 
     int enable = 1;
-    setsockopt(new_sock, IPPROTO_TCP, TCP_NODELAY, (const void *) &enable, sizeof(enable));
+    if (setsockopt(new_sock, IPPROTO_TCP, TCP_NODELAY,
+                   (const void *) &enable, sizeof(enable)) < 0) {
+        err(1, "setsockopt nodelay");
+    }
 
     // wait header
     wait_for(new_sock, POLLIN);
commit 982d14dd69a8e15e013f731df15908f5f4bea9a7
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Fri May 1 13:37:12 2020 +0100

    dispatcher: Move some private functions to DispatcherPrivate
    
    Reduce declaration in the header.
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Julien Ropé <jrope at gmail.com>

diff --git a/server/dispatcher.cpp b/server/dispatcher.cpp
index 0337dcb4..78726b2e 100644
--- a/server/dispatcher.cpp
+++ b/server/dispatcher.cpp
@@ -50,6 +50,10 @@ struct DispatcherPrivate {
     {
     }
     ~DispatcherPrivate();
+    void send_message(const DispatcherMessage& msg, void *payload);
+    bool handle_single_read();
+    static void handle_event(int fd, int event, DispatcherPrivate* priv);
+
     int recv_fd;
     int send_fd;
     pthread_mutex_t lock;
@@ -184,96 +188,89 @@ static int write_safe(int fd, uint8_t *buf, size_t size)
     return written_size;
 }
 
-int Dispatcher::handle_single_read(Dispatcher *dispatcher)
+bool DispatcherPrivate::handle_single_read()
 {
     int ret;
     DispatcherMessage msg[1];
-    void *payload;
     uint32_t ack = ACK;
 
-    if ((ret = read_safe(dispatcher->priv->recv_fd, (uint8_t*)msg, sizeof(msg), 0)) == -1) {
+    if ((ret = read_safe(recv_fd, (uint8_t*)msg, sizeof(msg), 0)) == -1) {
         g_warning("error reading from dispatcher: %d", errno);
-        return 0;
+        return false;
     }
     if (ret == 0) {
         /* no message */
-        return 0;
+        return false;
     }
-    if (G_UNLIKELY(msg->size > dispatcher->priv->payload_size)) {
-        dispatcher->priv->payload = g_realloc(dispatcher->priv->payload, msg->size);
-        dispatcher->priv->payload_size = msg->size;
+    if (G_UNLIKELY(msg->size > payload_size)) {
+        payload = g_realloc(payload, msg->size);
+        payload_size = msg->size;
     }
-    payload = dispatcher->priv->payload;
-    if (read_safe(dispatcher->priv->recv_fd, (uint8_t*) payload, msg->size, 1) == -1) {
+    if (read_safe(recv_fd, (uint8_t*) payload, msg->size, 1) == -1) {
         g_warning("error reading from dispatcher: %d", errno);
         /* TODO: close socketpair? */
-        return 0;
+        return false;
     }
-    if (dispatcher->priv->any_handler && msg->type != DISPATCHER_MESSAGE_TYPE_CUSTOM) {
-        dispatcher->priv->any_handler(dispatcher->priv->opaque, msg->type, payload);
+    if (any_handler && msg->type != DISPATCHER_MESSAGE_TYPE_CUSTOM) {
+        any_handler(opaque, msg->type, payload);
     }
     if (msg->handler) {
-        msg->handler(dispatcher->priv->opaque, payload);
+        msg->handler(opaque, payload);
     } else {
         g_warning("error: no handler for message type %d", msg->type);
     }
     if (msg->ack) {
-        if (write_safe(dispatcher->priv->recv_fd,
-                       (uint8_t*)&ack, sizeof(ack)) == -1) {
+        if (write_safe(recv_fd, (uint8_t*)&ack, sizeof(ack)) == -1) {
             g_warning("error writing ack for message %d", msg->type);
             /* TODO: close socketpair? */
         }
     }
-    return 1;
+    return true;
 }
 
 /*
  * handle_event
  * doesn't handle being in the middle of a message. all reads are blocking.
  */
-void Dispatcher::handle_event(int fd, int event, Dispatcher* dispatcher)
+void DispatcherPrivate::handle_event(int fd, int event, DispatcherPrivate* priv)
 {
-    while (dispatcher->handle_single_read(dispatcher)) {
+    while (priv->handle_single_read()) {
     }
 }
 
-void Dispatcher::send_message_internal(const DispatcherMessage* msg, void *payload)
+void DispatcherPrivate::send_message(const DispatcherMessage& msg, void *payload)
 {
     uint32_t ack;
-    int send_fd = priv->send_fd;
 
-    pthread_mutex_lock(&priv->lock);
-    if (write_safe(send_fd, (uint8_t*)msg, sizeof(*msg)) == -1) {
+    pthread_mutex_lock(&lock);
+    if (write_safe(send_fd, (uint8_t*)&msg, sizeof(msg)) == -1) {
         g_warning("error: failed to send message header for message %d",
-                  msg->type);
+                  msg.type);
         goto unlock;
     }
-    if (write_safe(send_fd, (uint8_t*) payload, msg->size) == -1) {
+    if (write_safe(send_fd, (uint8_t*) payload, msg.size) == -1) {
         g_warning("error: failed to send message body for message %d",
-                  msg->type);
+                  msg.type);
         goto unlock;
     }
-    if (msg->ack) {
+    if (msg.ack) {
         if (read_safe(send_fd, (uint8_t*)&ack, sizeof(ack), 1) == -1) {
             g_warning("error: failed to read ack");
         } else if (ack != ACK) {
             g_warning("error: got wrong ack value in dispatcher "
-                      "for message %d\n", msg->type);
+                      "for message %d\n", msg.type);
             /* TODO handling error? */
         }
     }
 unlock:
-    pthread_mutex_unlock(&priv->lock);
+    pthread_mutex_unlock(&lock);
 }
 
 void Dispatcher::send_message(uint32_t message_type, void *payload)
 {
-    DispatcherMessage *msg;
-
     assert(priv->max_message_type > message_type);
     assert(priv->messages[message_type].handler);
-    msg = &priv->messages[message_type];
-    send_message_internal(msg, payload);
+    priv->send_message(priv->messages[message_type], payload);
 }
 
 void Dispatcher::send_message_custom(dispatcher_handle_message handler,
@@ -285,7 +282,7 @@ void Dispatcher::send_message_custom(dispatcher_handle_message handler,
         .type = DISPATCHER_MESSAGE_TYPE_CUSTOM,
         .ack = ack,
     };
-    send_message_internal(&msg, payload);
+    priv->send_message(msg, payload);
 }
 
 void Dispatcher::register_handler(uint32_t message_type,
@@ -315,7 +312,7 @@ void Dispatcher::register_universal_handler(dispatcher_handle_any_message any_ha
 SpiceWatch *Dispatcher::create_watch(SpiceCoreInterfaceInternal *core)
 {
     return core->watch_new(priv->recv_fd,
-                           SPICE_WATCH_EVENT_READ, handle_event, this);
+                           SPICE_WATCH_EVENT_READ, DispatcherPrivate::handle_event, priv.get());
 }
 
 void Dispatcher::set_opaque(void *opaque)
diff --git a/server/dispatcher.h b/server/dispatcher.h
index 232d9a92..819866b1 100644
--- a/server/dispatcher.h
+++ b/server/dispatcher.h
@@ -153,9 +153,6 @@ protected:
     virtual ~Dispatcher();
 
 private:
-    static int handle_single_read(Dispatcher *dispatcher);
-    static void handle_event(int fd, int event, Dispatcher* dispatcher);
-    void send_message_internal(const DispatcherMessage*msg, void *payload);
     red::unique_link<DispatcherPrivate> priv;
 };
 
diff --git a/server/utils.hpp b/server/utils.hpp
index 854d781f..eac4191a 100644
--- a/server/utils.hpp
+++ b/server/utils.hpp
@@ -66,6 +66,10 @@ public:
     {
         return p;
     }
+    T *get() const noexcept
+    {
+        return p;
+    }
 private:
     T *const p;
     unique_link(const unique_link&)=delete;
commit b8f4d7d2c7a3d08a82f4bc7588cdff15cee54292
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Tue Jun 16 11:49:19 2020 +0100

    websocket: Fix possible integer overflow
    
    The shift of a uint_8 number by a number > 32 causes an overflow.
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Uri Lublin <ulublin at redhat.com>

diff --git a/server/websocket.c b/server/websocket.c
index f5df63f8..82b20b49 100644
--- a/server/websocket.c
+++ b/server/websocket.c
@@ -165,8 +165,9 @@ static uint64_t extract_length(const uint8_t *buf, int *used)
     case LENGTH_64BIT:
         *used += 8;
         outlen = 0;
-        for (i = 56; i >= 0; i -= 8) {
-            outlen |= (*buf++) << i;
+        for (i = 0; i < 8; ++i) {
+            outlen <<= 8;
+            outlen |= *buf++;
         }
         break;
 
commit 70347f31759f05f78a41f4f788801a1a705e6add
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Sat Jun 6 08:25:49 2020 +0100

    dcc: Remove unused result from dcc_add_surface_area_image
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Julien Ropé <jrope at redhat.com>

diff --git a/server/dcc.cpp b/server/dcc.cpp
index 788deb90..3fd58535 100644
--- a/server/dcc.cpp
+++ b/server/dcc.cpp
@@ -198,11 +198,9 @@ void dcc_create_surface(DisplayChannelClient *dcc, int surface_id)
 }
 
 // adding the pipe item after pos. If pos == NULL, adding to head.
-RedImageItem *dcc_add_surface_area_image(DisplayChannelClient *dcc,
-                                         int surface_id,
-                                         SpiceRect *area,
-                                         GList *pipe_item_pos,
-                                         int can_lossy)
+void
+dcc_add_surface_area_image(DisplayChannelClient *dcc, int surface_id,
+                           SpiceRect *area, GList *pipe_item_pos, int can_lossy)
 {
     DisplayChannel *display = DCC_TO_DC(dcc);
     RedSurface *surface = &display->priv->surfaces[surface_id];
@@ -256,8 +254,6 @@ RedImageItem *dcc_add_surface_area_image(DisplayChannelClient *dcc,
     } else {
         dcc->pipe_add(&item->base);
     }
-
-    return item;
 }
 
 void dcc_push_surface_image(DisplayChannelClient *dcc, int surface_id)
diff --git a/server/dcc.h b/server/dcc.h
index 8999bf7c..3172ba1b 100644
--- a/server/dcc.h
+++ b/server/dcc.h
@@ -152,11 +152,6 @@ void                       dcc_create_surface                        (DisplayCha
                                                                       int surface_id);
 void                       dcc_push_surface_image                    (DisplayChannelClient *dcc,
                                                                       int surface_id);
-RedImageItem *             dcc_add_surface_area_image                (DisplayChannelClient *dcc,
-                                                                      int surface_id,
-                                                                      SpiceRect *area,
-                                                                      GList *pipe_item_pos,
-                                                                      int can_lossy);
 void                       dcc_palette_cache_reset                   (DisplayChannelClient *dcc);
 void                       dcc_palette_cache_palette                 (DisplayChannelClient *dcc,
                                                                       SpicePalette *palette,
@@ -185,6 +180,8 @@ int                        dcc_compress_image                        (DisplayCha
                                                                       int can_lossy,
                                                                       compress_send_data_t* o_comp_data);
 
+void dcc_add_surface_area_image(DisplayChannelClient *dcc, int surface_id,
+                                SpiceRect *area, GList *pipe_item_pos, int can_lossy);
 VideoStreamAgent *dcc_get_video_stream_agent(DisplayChannelClient *dcc, int stream_id);
 ImageEncoders *dcc_get_encoders(DisplayChannelClient *dcc);
 spice_wan_compression_t    dcc_get_jpeg_state                        (DisplayChannelClient *dcc);
commit 87b6b1993e4e17f8e2eadaa787aa5219730a7510
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Tue May 5 03:34:12 2020 +0100

    sound: Make receive_buf field private
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>

diff --git a/server/sound.cpp b/server/sound.cpp
index 859c9c8b..9f9aed0b 100644
--- a/server/sound.cpp
+++ b/server/sound.cpp
@@ -90,9 +90,6 @@ public:
 
     uint32_t command;
 
-    /* we don't expect very big messages so don't allocate too much
-     * bytes, data will be cached in RecordChannelClient::samples */
-    uint8_t receive_buf[SND_CODEC_MAX_FRAME_BYTES + 64];
     PersistentPipeItem persistent_pipe_item;
 
     virtual void on_message_done() {};
@@ -103,6 +100,11 @@ public:
     virtual uint8_t *alloc_recv_buf(uint16_t type, uint32_t size) override;
     virtual void release_recv_buf(uint16_t type, uint32_t size, uint8_t *msg) override;
     virtual void migrate() override;
+
+private:
+    /* we don't expect very big messages so don't allocate too much
+     * bytes, data will be cached in RecordChannelClient::samples */
+    uint8_t receive_buf[SND_CODEC_MAX_FRAME_BYTES + 64];
 };
 
 static void snd_playback_alloc_frames(PlaybackChannelClient *playback);
