From b174e757faa2e678ee6c56b33f9d68322984c5d3 Mon Sep 17 00:00:00 2001 From: Frediano Ziglio Date: Thu, 4 Jun 2020 06:43:03 +0100 Subject: [PATCH] Enable -Wshadow warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This flag allows to catch variables or arguments hiding other variables or attributes. It helps avoiding some possible mistakes. Signed-off-by: Frediano Ziglio Acked-by: Julien Ropé --- m4/spice-compile-warnings.m4 | 3 +++ meson.build | 1 + server/dispatcher.cpp | 8 ++++---- server/display-channel.cpp | 12 +++++------- server/inputs-channel.cpp | 16 ++++++++-------- server/inputs-channel.h | 2 +- server/main-channel.cpp | 8 ++++---- server/main-dispatcher.cpp | 4 ++-- server/red-channel-client.cpp | 14 +++++++------- server/red-channel.cpp | 15 ++++++++------- server/red-client.cpp | 4 ++-- server/red-replay-qxl.cpp | 6 +++--- server/red-stream-device.cpp | 6 +++--- server/safe-list.hpp | 6 +++--- server/spicevmc.cpp | 14 +++++++------- server/tests/test-display-base.cpp | 9 +++++---- server/tests/test-smartcard.cpp | 10 +++++----- server/utils.hpp | 10 +++++----- 18 files changed, 76 insertions(+), 72 deletions(-) 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 ba28e7c2..8aab78a5 100644 --- a/meson.build +++ b/meson.build @@ -199,6 +199,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..0ddb63ae 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 init_max_message_type): + max_message_type(init_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 384cde6b..5261b348 100644 --- a/server/display-channel.cpp +++ b/server/display-channel.cpp @@ -322,9 +322,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++; @@ -339,8 +338,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; @@ -760,9 +758,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..533f679c 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 *init_reds): Dispatcher(MAIN_DISPATCHER_NUM_MESSAGES), - reds(reds), + reds(init_reds), thread_id(pthread_self()) { set_opaque(reds); diff --git a/server/red-channel-client.cpp b/server/red-channel-client.cpp index b13fe1ab..38934db5 100644 --- a/server/red-channel-client.cpp +++ b/server/red-channel-client.cpp @@ -291,14 +291,14 @@ void RedChannelClientPrivate::restart_ping_timer() start_ping_timer(timeout); } -RedChannelClientPrivate::RedChannelClientPrivate(RedChannel *channel, - RedClient *client, - RedStream *stream, +RedChannelClientPrivate::RedChannelClientPrivate(RedChannel *init_channel, + RedClient *init_client, + RedStream *init_stream, RedChannelCapabilities *caps, - bool monitor_latency): - channel(channel), - client(client), stream(stream), - monitor_latency(monitor_latency) + bool init_monitor_latency): + channel(init_channel), + client(init_client), stream(init_stream), + monitor_latency(init_monitor_latency) { // 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..38795e13 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 *init_reds, uint32_t init_type, uint32_t init_id, + RedChannel::CreationFlags flags, + SpiceCoreInterfaceInternal *init_core, Dispatcher *init_dispatcher): + type(init_type), id(init_id), + core(init_core ? init_core : reds_get_core_interface(init_reds)), handle_acks(!!(flags & RedChannel::HandleAcks)), - parser(spice_get_client_channel_parser(type, nullptr)), + parser(spice_get_client_channel_parser(init_type, nullptr)), migration_flags(flags & RedChannel::MigrateAll), - dispatcher(dispatcher), - reds(reds) + dispatcher(init_dispatcher), + reds(init_reds) { thread_id = pthread_self(); } diff --git a/server/red-client.cpp b/server/red-client.cpp index 04ce7113..06a55aaa 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 *init_reds, bool migrated): + reds(init_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 c62ceceb..d21c6e77 100644 --- a/server/red-stream-device.cpp +++ b/server/red-stream-device.cpp @@ -186,10 +186,10 @@ StreamDevice::handle_msg_invalid(const char *error_msg) 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..960f21a3 100644 --- a/server/safe-list.hpp +++ b/server/safe-list.hpp @@ -110,9 +110,9 @@ class safe_list::iterator: public std::iterator typedef typename std::forward_list>::iterator wrapped; wrapped curr, next; public: - iterator(wrapped curr) : - curr(curr), - next(curr != wrapped() ? ++curr : wrapped()) + iterator(wrapped init_curr) : + curr(init_curr), + next(init_curr != wrapped() ? ++init_curr : wrapped()) { } iterator& operator++() diff --git a/server/spicevmc.cpp b/server/spicevmc.cpp index 0c187c3f..8f1d8cfb 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; @@ -619,21 +620,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(); @@ -685,9 +685,9 @@ void RedCharDeviceSpiceVmc::port_event(uint8_t event) } RedCharDeviceSpiceVmc::RedCharDeviceSpiceVmc(SpiceCharDeviceInstance *sin, RedsState *reds, - RedVmcChannel *channel): + RedVmcChannel *init_channel): RedCharDevice(reds, sin, 0, 128), - channel(channel) + channel(init_channel) { if (channel) { channel->chardev = this; diff --git a/server/tests/test-display-base.cpp b/server/tests/test-display-base.cpp index e53d4843..70814f1f 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 e1afdf98..063e3f62 100644 --- a/server/utils.hpp +++ b/server/utils.hpp @@ -54,7 +54,7 @@ public: unique_link(): p(new T()) { } - unique_link(T* p): p(p) + unique_link(T* ptr): p(ptr) { } ~unique_link() @@ -137,14 +137,14 @@ class shared_ptr { friend class weak_ptr; 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 - explicit shared_ptr(Q *p): shared_ptr(static_cast(p)) + explicit shared_ptr(Q *ptr): shared_ptr(static_cast(ptr)) { } shared_ptr(const shared_ptr& rhs): p(rhs.p) @@ -218,7 +218,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) { } }; @@ -299,7 +299,7 @@ template 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);