From 431208ab76fcda069580f1ba7e7275afef807b3f Mon Sep 17 00:00:00 2001 From: William Woodall Date: Tue, 5 Jan 2016 13:20:07 -0800 Subject: [PATCH 1/3] fix different signed comparison warnings --- rcl/test/rcl/test_allocator.cpp | 10 +++---- rcl/test/rcl/test_node.cpp | 12 ++++----- rcl/test/rcl/test_rcl.cpp | 12 ++++----- rcl/test/rcl/test_time.cpp | 4 +-- rcl/test/test_memory_tools.cpp | 48 ++++++++++++++++----------------- 5 files changed, 43 insertions(+), 43 deletions(-) diff --git a/rcl/test/rcl/test_allocator.cpp b/rcl/test/rcl/test_allocator.cpp index b9ca12a..2216c54 100644 --- a/rcl/test/rcl/test_allocator.cpp +++ b/rcl/test/rcl/test_allocator.cpp @@ -72,13 +72,13 @@ TEST_F(TestAllocatorFixture, test_default_allocator_normal) { assert_no_realloc_begin(); assert_no_free_begin(); void * allocated_memory = allocator.allocate(1024, allocator.state); - EXPECT_EQ(mallocs, 1); + EXPECT_EQ(mallocs, 1u); EXPECT_NE(allocated_memory, nullptr); allocated_memory = allocator.reallocate(allocated_memory, 2048, allocator.state); - EXPECT_EQ(reallocs, 1); + EXPECT_EQ(reallocs, 1u); EXPECT_NE(allocated_memory, nullptr); allocator.deallocate(allocated_memory, allocator.state); - EXPECT_EQ(mallocs, 1); - EXPECT_EQ(reallocs, 1); - EXPECT_EQ(frees, 1); + EXPECT_EQ(mallocs, 1u); + EXPECT_EQ(reallocs, 1u); + EXPECT_EQ(frees, 1u); } diff --git a/rcl/test/rcl/test_node.cpp b/rcl/test/rcl/test_node.cpp index 0fa6b15..253956e 100644 --- a/rcl/test/rcl/test_node.cpp +++ b/rcl/test/rcl/test_node.cpp @@ -201,7 +201,7 @@ TEST_F(TestNodeFixture, test_rcl_node_accessors) { EXPECT_EQ(RCL_RET_OK, ret); if (RCL_RET_OK == ret && (!is_windows || !is_opensplice)) { // Can only expect the domain id to be 42 if not windows or not opensplice. - EXPECT_EQ(42, actual_domain_id); + EXPECT_EQ(42u, actual_domain_id); } // Test rcl_node_get_rmw_handle(). rmw_node_t * node_handle; @@ -227,14 +227,14 @@ TEST_F(TestNodeFixture, test_rcl_node_accessors) { // Test rcl_node_get_rcl_instance_id(). uint64_t instance_id; instance_id = rcl_node_get_rcl_instance_id(nullptr); - EXPECT_EQ(0, instance_id); + EXPECT_EQ(0u, instance_id); rcl_reset_error(); instance_id = rcl_node_get_rcl_instance_id(&zero_node); - EXPECT_EQ(0, instance_id); + EXPECT_EQ(0u, instance_id); rcl_reset_error(); instance_id = rcl_node_get_rcl_instance_id(&invalid_node); - EXPECT_NE(0, instance_id); - EXPECT_NE(42, instance_id); + EXPECT_NE(0u, instance_id); + EXPECT_NE(42u, instance_id); rcl_reset_error(); start_memory_checking(); assert_no_malloc_begin(); @@ -245,7 +245,7 @@ TEST_F(TestNodeFixture, test_rcl_node_accessors) { assert_no_realloc_end(); assert_no_free_end(); stop_memory_checking(); - EXPECT_NE(0, instance_id); + EXPECT_NE(0u, instance_id); } /* Tests the node life cycle, including rcl_node_init() and rcl_node_fini(). diff --git a/rcl/test/rcl/test_rcl.cpp b/rcl/test/rcl/test_rcl.cpp index 7b8d0af..981aee2 100644 --- a/rcl/test/rcl/test_rcl.cpp +++ b/rcl/test/rcl/test_rcl.cpp @@ -184,12 +184,12 @@ TEST_F(TestRCLFixture, test_rcl_init_and_ok_and_shutdown) { TEST_F(TestRCLFixture, test_rcl_get_instance_id_and_ok) { rcl_ret_t ret; // Instance id should be 0 before rcl_init(). - EXPECT_EQ(0, rcl_get_instance_id()); + EXPECT_EQ(0u, rcl_get_instance_id()); ASSERT_FALSE(rcl_ok()); // It should still return 0 after an invalid init. ret = rcl_init(1, nullptr, rcl_get_default_allocator()); EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret); - EXPECT_EQ(0, rcl_get_instance_id()); + EXPECT_EQ(0u, rcl_get_instance_id()); ASSERT_FALSE(rcl_ok()); // A non-zero instance id should be returned after a valid init. { @@ -207,13 +207,13 @@ TEST_F(TestRCLFixture, test_rcl_get_instance_id_and_ok) { assert_no_malloc_end(); assert_no_realloc_end(); assert_no_free_end(); - EXPECT_NE(0, first_instance_id); + EXPECT_NE(0u, first_instance_id); EXPECT_EQ(first_instance_id, rcl_get_instance_id()); // Repeat calls should return the same. EXPECT_EQ(true, rcl_ok()); // Calling after a shutdown should return 0. ret = rcl_shutdown(); EXPECT_EQ(ret, RCL_RET_OK); - EXPECT_EQ(0, rcl_get_instance_id()); + EXPECT_EQ(0u, rcl_get_instance_id()); ASSERT_FALSE(rcl_ok()); // It should return a different value after another valid init. { @@ -222,12 +222,12 @@ TEST_F(TestRCLFixture, test_rcl_get_instance_id_and_ok) { EXPECT_EQ(RCL_RET_OK, ret); ASSERT_TRUE(rcl_ok()); } - EXPECT_NE(0, rcl_get_instance_id()); + EXPECT_NE(0u, rcl_get_instance_id()); EXPECT_NE(first_instance_id, rcl_get_instance_id()); ASSERT_TRUE(rcl_ok()); // Shutting down a second time should result in 0 again. ret = rcl_shutdown(); EXPECT_EQ(ret, RCL_RET_OK); - EXPECT_EQ(0, rcl_get_instance_id()); + EXPECT_EQ(0u, rcl_get_instance_id()); ASSERT_FALSE(rcl_ok()); } diff --git a/rcl/test/rcl/test_time.cpp b/rcl/test/rcl/test_time.cpp index 1e89f10..5d68ee9 100644 --- a/rcl/test/rcl/test_time.cpp +++ b/rcl/test/rcl/test_time.cpp @@ -66,7 +66,7 @@ TEST_F(TestTimeFixture, test_rcl_system_time_point_now) { assert_no_free_end(); stop_memory_checking(); EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string_safe(); - EXPECT_NE(now.nanoseconds, 0); + EXPECT_NE(now.nanoseconds, 0u); // Compare to std::chrono::system_clock time (within a second). now = {0}; ret = rcl_system_time_point_now(&now); @@ -99,7 +99,7 @@ TEST_F(TestTimeFixture, test_rcl_steady_time_point_now) { assert_no_free_end(); stop_memory_checking(); EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string_safe(); - EXPECT_NE(now.nanoseconds, 0); + EXPECT_NE(now.nanoseconds, 0u); // Compare to std::chrono::steady_clock difference of two times (within a second). now = {0}; ret = rcl_steady_time_point_now(&now); diff --git a/rcl/test/test_memory_tools.cpp b/rcl/test/test_memory_tools.cpp index dcddaa3..ecf3376 100644 --- a/rcl/test/test_memory_tools.cpp +++ b/rcl/test/test_memory_tools.cpp @@ -43,9 +43,9 @@ TEST(TestMemoryTools, test_allocation_checking_tools) { ASSERT_NE(remem, nullptr); if (!remem) {free(mem);} free(remem); - EXPECT_EQ(unexpected_mallocs, 0); - EXPECT_EQ(unexpected_reallocs, 0); - EXPECT_EQ(unexpected_frees, 0); + EXPECT_EQ(unexpected_mallocs, 0u); + EXPECT_EQ(unexpected_reallocs, 0u); + EXPECT_EQ(unexpected_frees, 0u); // Enable checking, but no assert, should have no effect. start_memory_checking(); mem = malloc(1024); @@ -54,9 +54,9 @@ TEST(TestMemoryTools, test_allocation_checking_tools) { ASSERT_NE(remem, nullptr); if (!remem) {free(mem);} free(remem); - EXPECT_EQ(unexpected_mallocs, 0); - EXPECT_EQ(unexpected_reallocs, 0); - EXPECT_EQ(unexpected_frees, 0); + EXPECT_EQ(unexpected_mallocs, 0u); + EXPECT_EQ(unexpected_reallocs, 0u); + EXPECT_EQ(unexpected_frees, 0u); // Enable no_* asserts, should increment all once. assert_no_malloc_begin(); assert_no_realloc_begin(); @@ -70,9 +70,9 @@ TEST(TestMemoryTools, test_allocation_checking_tools) { if (!remem) {free(mem);} free(remem); assert_no_free_end(); - EXPECT_EQ(unexpected_mallocs, 1); - EXPECT_EQ(unexpected_reallocs, 1); - EXPECT_EQ(unexpected_frees, 1); + EXPECT_EQ(unexpected_mallocs, 1u); + EXPECT_EQ(unexpected_reallocs, 1u); + EXPECT_EQ(unexpected_frees, 1u); // Enable on malloc assert, only malloc should increment. assert_no_malloc_begin(); mem = malloc(1024); @@ -82,9 +82,9 @@ TEST(TestMemoryTools, test_allocation_checking_tools) { ASSERT_NE(remem, nullptr); if (!remem) {free(mem);} free(remem); - EXPECT_EQ(unexpected_mallocs, 2); - EXPECT_EQ(unexpected_reallocs, 1); - EXPECT_EQ(unexpected_frees, 1); + EXPECT_EQ(unexpected_mallocs, 2u); + EXPECT_EQ(unexpected_reallocs, 1u); + EXPECT_EQ(unexpected_frees, 1u); // Enable on realloc assert, only realloc should increment. assert_no_realloc_begin(); mem = malloc(1024); @@ -94,9 +94,9 @@ TEST(TestMemoryTools, test_allocation_checking_tools) { ASSERT_NE(remem, nullptr); if (!remem) {free(mem);} free(remem); - EXPECT_EQ(unexpected_mallocs, 2); - EXPECT_EQ(unexpected_reallocs, 2); - EXPECT_EQ(unexpected_frees, 1); + EXPECT_EQ(unexpected_mallocs, 2u); + EXPECT_EQ(unexpected_reallocs, 2u); + EXPECT_EQ(unexpected_frees, 1u); // Enable on free assert, only free should increment. assert_no_free_begin(); mem = malloc(1024); @@ -106,9 +106,9 @@ TEST(TestMemoryTools, test_allocation_checking_tools) { if (!remem) {free(mem);} free(remem); assert_no_free_end(); - EXPECT_EQ(unexpected_mallocs, 2); - EXPECT_EQ(unexpected_reallocs, 2); - EXPECT_EQ(unexpected_frees, 2); + EXPECT_EQ(unexpected_mallocs, 2u); + EXPECT_EQ(unexpected_reallocs, 2u); + EXPECT_EQ(unexpected_frees, 2u); // Go again, after disabling asserts, should have no effect. mem = malloc(1024); ASSERT_NE(mem, nullptr); @@ -116,9 +116,9 @@ TEST(TestMemoryTools, test_allocation_checking_tools) { ASSERT_NE(remem, nullptr); if (!remem) {free(mem);} free(remem); - EXPECT_EQ(unexpected_mallocs, 2); - EXPECT_EQ(unexpected_reallocs, 2); - EXPECT_EQ(unexpected_frees, 2); + EXPECT_EQ(unexpected_mallocs, 2u); + EXPECT_EQ(unexpected_reallocs, 2u); + EXPECT_EQ(unexpected_frees, 2u); // Go once more after disabling everything, should have no effect. stop_memory_checking(); mem = malloc(1024); @@ -127,7 +127,7 @@ TEST(TestMemoryTools, test_allocation_checking_tools) { ASSERT_NE(remem, nullptr); if (!remem) {free(mem);} free(remem); - EXPECT_EQ(unexpected_mallocs, 2); - EXPECT_EQ(unexpected_reallocs, 2); - EXPECT_EQ(unexpected_frees, 2); + EXPECT_EQ(unexpected_mallocs, 2u); + EXPECT_EQ(unexpected_reallocs, 2u); + EXPECT_EQ(unexpected_frees, 2u); } From a35596a4aba2b690e087b7f129581e61afe8bec2 Mon Sep 17 00:00:00 2001 From: William Woodall Date: Tue, 5 Jan 2016 14:50:29 -0800 Subject: [PATCH 2/3] suppress unused variable when compiling with RelWithDebInfo --- rcl/src/rcl/wait.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rcl/src/rcl/wait.c b/rcl/src/rcl/wait.c index eb87cdc..5880745 100644 --- a/rcl/src/rcl/wait.c +++ b/rcl/src/rcl/wait.c @@ -64,6 +64,7 @@ static void __wait_set_clean_up(rcl_wait_set_t * wait_set, rcl_allocator_t allocator) { rcl_ret_t ret; + (void)ret; // NO LINT if (wait_set->subscriptions) { ret = rcl_wait_set_resize_subscriptions(wait_set, 0); assert(ret == RCL_RET_OK); // Defensive, shouldn't fail with size 0. @@ -429,6 +430,7 @@ rcl_wait(rcl_wait_set_t * wait_set, int64_t timeout) if (ret == RMW_RET_TIMEOUT) { // Assume none were set (because timeout was reached first), and clear all. rcl_ret_t rcl_ret; + (void)rcl_ret; // NO LINT rcl_ret = rcl_wait_set_clear_subscriptions(wait_set); assert(rcl_ret == RCL_RET_OK); // Defensive, shouldn't fail with valid wait_set. rcl_ret = rcl_wait_set_clear_guard_conditions(wait_set); From 33103599283579dca928779686ddb6f8fd4c607c Mon Sep 17 00:00:00 2001 From: William Woodall Date: Tue, 5 Jan 2016 15:42:10 -0800 Subject: [PATCH 3/3] reduce scope of variable --- rcl/src/rcl/wait.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/rcl/src/rcl/wait.c b/rcl/src/rcl/wait.c index 5880745..33ebf14 100644 --- a/rcl/src/rcl/wait.c +++ b/rcl/src/rcl/wait.c @@ -63,18 +63,19 @@ __wait_set_is_valid(const rcl_wait_set_t * wait_set) static void __wait_set_clean_up(rcl_wait_set_t * wait_set, rcl_allocator_t allocator) { - rcl_ret_t ret; - (void)ret; // NO LINT if (wait_set->subscriptions) { - ret = rcl_wait_set_resize_subscriptions(wait_set, 0); + rcl_ret_t ret = rcl_wait_set_resize_subscriptions(wait_set, 0); + (void)ret; // NO LINT assert(ret == RCL_RET_OK); // Defensive, shouldn't fail with size 0. } if (wait_set->guard_conditions) { - ret = rcl_wait_set_resize_guard_conditions(wait_set, 0); + rcl_ret_t ret = rcl_wait_set_resize_guard_conditions(wait_set, 0); + (void)ret; // NO LINT assert(ret == RCL_RET_OK); // Defensive, shouldn't fail with size 0. } if (wait_set->timers) { - ret = rcl_wait_set_resize_timers(wait_set, 0); + rcl_ret_t ret = rcl_wait_set_resize_timers(wait_set, 0); + (void)ret; // NO LINT assert(ret == RCL_RET_OK); // Defensive, shouldn't fail with size 0. } if (wait_set->impl) {