From b286d1c1ba69f4a46c06e7f75d2a0284dae02308 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Fri, 29 Apr 2022 17:56:40 -0700 Subject: [PATCH] tests: fix compile time warnings test/syscalls/linux/futex.cc: In member function 'virtual void gvisor::testing:: {anonymous}::PrivateAndSharedFutexTest_PIWaiters_Test::TestBody()': test/syscalls/linux/futex.cc:697:19: warning: comparison of integer expressions of different signedness: 'std::__atomic_base::__int_type' {aka 'int'} and 'unsigned int' [-Wsign-compare] 697 | while (a.load() != (FUTEX_WAITERS | gettid())) { | ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ test/syscalls/linux/proc_pid_uid_gid_map.cc:207:64: warning: comparison of integer expressions of different signedness: 'size_t' {aka 'long unsigned int'} and 'int' [-Wsign-compare] 207 | TEST_PCHECK((n = write(fd, line.c_str(), line.size())) != -1); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~ test/syscalls/linux/socket_inet_loopback.cc:964:21: warning: comparison of integer expressions of different signedness: 'int' and 'std::array::size_type' {aka 'long unsigned int'} [-Wsign-compare] 964 | for (int i = 0; i < std::size(established_clients); i++) { | ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ test/syscalls/linux/socket_inet_loopback.cc:974:21: warning: comparison of integer expressions of different signedness: 'int' and 'std::array::size_type' {aka 'long unsigned int'} [-Wsign-compare] 974 | for (int i = 0; i < std::size(waiting_clients); i++) { | ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~ test/syscalls/linux/udp_socket.cc:869:6: warning: suggest explicit braces to avoid ambiguous 'else' [-Wdangling-else] 869 |if (!IsRunningWithHostinet() || GvisorPlatform() == Platform::kPtrace || | ^ test/syscalls/linux/socket_unix_unbound_abstract.cc:93:9: warning: variable 'orig_opts' set but not used [-Wunused-but-set-variable] 93 | int orig_opts; | ^~~~~~~~~ test/syscalls/linux/socket_unix_unbound_abstract.cc:107:9: warning: variable 'orig_opts' set but not used [-Wunused-but-set-variable] 107 | int orig_opts; | ^~~~~~~~~ PiperOrigin-RevId: 445545240 --- test/perf/linux/fork_benchmark.cc | 8 ++++---- test/perf/linux/futex_benchmark.cc | 2 +- test/perf/linux/pipe_benchmark.cc | 2 +- test/perf/linux/seqwrite_benchmark.cc | 2 +- test/syscalls/linux/proc_pid_uid_gid_map.cc | 8 ++++---- test/syscalls/linux/socket_generic_stress.cc | 6 +++--- test/syscalls/linux/socket_inet_loopback.cc | 4 ++-- test/syscalls/linux/socket_unix_unbound_abstract.cc | 4 ---- test/syscalls/linux/udp_socket.cc | 7 +------ 9 files changed, 17 insertions(+), 26 deletions(-) diff --git a/test/perf/linux/fork_benchmark.cc b/test/perf/linux/fork_benchmark.cc index 84fdbc8a0..1e11825ec 100644 --- a/test/perf/linux/fork_benchmark.cc +++ b/test/perf/linux/fork_benchmark.cc @@ -58,7 +58,7 @@ void BM_CPUBoundAsymmetric(benchmark::State& state) { const size_t max = state.max_iterations; pid_t child = fork(); if (child == 0) { - for (int i = 0; i < max; i++) { + for (size_t i = 0; i < max; i++) { busy(kBusyMax); } _exit(0); @@ -95,7 +95,7 @@ void BM_CPUBoundSymmetric(benchmark::State& state) { } pid_t child = fork(); if (child == 0) { - for (int i = 0; i < cur; i++) { + for (size_t i = 0; i < cur; i++) { busy(kBusyMax); } _exit(0); @@ -296,7 +296,7 @@ void BM_ThreadStart(benchmark::State& state) { state.ResumeTiming(); - for (size_t i = 0; i < num_threads; ++i) { + for (int i = 0; i < num_threads; ++i) { threads.emplace_back(std::make_unique([barrier] { if (barrier->Block()) { delete barrier; @@ -326,7 +326,7 @@ void BM_ProcessLifecycle(benchmark::State& state) { std::vector pids(num_procs); for (auto _ : state) { - for (size_t i = 0; i < num_procs; ++i) { + for (int i = 0; i < num_procs; ++i) { int pid = fork(); if (pid == 0) { _exit(0); diff --git a/test/perf/linux/futex_benchmark.cc b/test/perf/linux/futex_benchmark.cc index e686041c9..5f83793b8 100644 --- a/test/perf/linux/futex_benchmark.cc +++ b/test/perf/linux/futex_benchmark.cc @@ -162,7 +162,7 @@ void BM_FutexRoundtripDelayed(benchmark::State& state) { constexpr int64_t kBeforeWakeDelayNs = 500; std::atomic v(0); ScopedThread t([&] { - for (int i = 0; i < state.max_iterations; i++) { + for (benchmark::IterationCount i = 0; i < state.max_iterations; i++) { SpinNanos(delay_ns); while (v.load(std::memory_order_acquire) == 0) { FutexWait(&v, 0); diff --git a/test/perf/linux/pipe_benchmark.cc b/test/perf/linux/pipe_benchmark.cc index 8f5f6a2a3..295316bf2 100644 --- a/test/perf/linux/pipe_benchmark.cc +++ b/test/perf/linux/pipe_benchmark.cc @@ -40,7 +40,7 @@ void BM_Pipe(benchmark::State& state) { ScopedThread t([&] { auto const fd = fds[1]; - for (int i = 0; i < state.max_iterations; i++) { + for (benchmark::IterationCount i = 0; i < state.max_iterations; i++) { TEST_CHECK(WriteFd(fd, wbuf.data(), wbuf.size()) == size); } }); diff --git a/test/perf/linux/seqwrite_benchmark.cc b/test/perf/linux/seqwrite_benchmark.cc index af49e4477..af5e73e7f 100644 --- a/test/perf/linux/seqwrite_benchmark.cc +++ b/test/perf/linux/seqwrite_benchmark.cc @@ -46,7 +46,7 @@ void BM_SeqWrite(benchmark::State& state) { uint64_t offset = 0; for (auto _ : state) { TEST_CHECK(PwriteFd(fd.get(), buf.data(), buf.size(), offset) == - buf.size()); + ssize_t(buf.size())); offset += buf.size(); // Wrap around if going above the maximum file size. if (offset >= kMaxFile) { diff --git a/test/syscalls/linux/proc_pid_uid_gid_map.cc b/test/syscalls/linux/proc_pid_uid_gid_map.cc index c030592c8..150352df5 100644 --- a/test/syscalls/linux/proc_pid_uid_gid_map.cc +++ b/test/syscalls/linux/proc_pid_uid_gid_map.cc @@ -203,9 +203,9 @@ TEST_P(ProcSelfUidGidMapTest, IdentityMapOwnID) { EXPECT_THAT( InNewUserNamespaceWithMapFD([&](int fd) { DenySelfSetgroups(); - size_t n; + ssize_t n; TEST_PCHECK((n = write(fd, line.c_str(), line.size())) != -1); - TEST_CHECK(n == line.size()); + TEST_CHECK(n == ssize_t(line.size())); }), IsPosixErrorOkAndHolds(0)); } @@ -222,9 +222,9 @@ TEST_P(ProcSelfUidGidMapTest, TrailingNewlineAndNULIgnored) { DenySelfSetgroups(); // The write should return the full size of the write, even though // characters after the NUL were ignored. - size_t n; + ssize_t n; TEST_PCHECK((n = write(fd, line.c_str(), line.size())) != -1); - TEST_CHECK(n == line.size()); + TEST_CHECK(n == ssize_t(line.size())); }), IsPosixErrorOkAndHolds(0)); } diff --git a/test/syscalls/linux/socket_generic_stress.cc b/test/syscalls/linux/socket_generic_stress.cc index 9ff385b41..c08e4d99e 100644 --- a/test/syscalls/linux/socket_generic_stress.cc +++ b/test/syscalls/linux/socket_generic_stress.cc @@ -209,7 +209,7 @@ TEST_P(DataTransferStressTest, BigDataTransfer) { if (r == 0) { break; } - for (size_t i = 0; i < r;) { + for (ssize_t i = 0; i < r;) { ssize_t w = write(server_fd, buf.data() + i, r - i); ASSERT_GE(w, 0); i += w; @@ -240,8 +240,8 @@ TEST_P(DataTransferStressTest, BigDataTransfer) { while (!data.empty()) { ssize_t n = read(client_fd, buf.data(), buf.size()); ASSERT_GE(n, 0); - for (size_t i = 0; i < n; i += chunk.size()) { - size_t c = std::min(chunk.size(), n - i); + for (ssize_t i = 0; i < n; i += chunk.size()) { + ssize_t c = std::min(ssize_t(chunk.size()), n - i); ASSERT_EQ(buf.substr(i, c), data.substr(i, c)) << "offset " << i; } data = data.substr(n); diff --git a/test/syscalls/linux/socket_inet_loopback.cc b/test/syscalls/linux/socket_inet_loopback.cc index 777487839..0ba88735f 100644 --- a/test/syscalls/linux/socket_inet_loopback.cc +++ b/test/syscalls/linux/socket_inet_loopback.cc @@ -961,7 +961,7 @@ TEST_P(SocketInetLoopbackTest, TCPBacklogAcceptAll) { // Ensure that we accept all client connections. The waiting connections would // get enqueued as we drain the accept queue. - for (int i = 0; i < std::size(established_clients); i++) { + for (std::size_t i = 0; i < std::size(established_clients); i++) { SCOPED_TRACE(absl::StrCat("established clients i=", i)); accept_connection(); } @@ -971,7 +971,7 @@ TEST_P(SocketInetLoopbackTest, TCPBacklogAcceptAll) { // (2) ESTABLISHED: if the listener sent back a SYNACK, but may have dropped // the ACK from the client if the accept queue was full (send out a data to // re-send that ACK, to address that case). - for (int i = 0; i < std::size(waiting_clients); i++) { + for (std::size_t i = 0; i < std::size(waiting_clients); i++) { SCOPED_TRACE(absl::StrCat("waiting clients i=", i)); constexpr int kTimeout = 10000; pollfd pfd = { diff --git a/test/syscalls/linux/socket_unix_unbound_abstract.cc b/test/syscalls/linux/socket_unix_unbound_abstract.cc index 0f6864266..3a91e97ac 100644 --- a/test/syscalls/linux/socket_unix_unbound_abstract.cc +++ b/test/syscalls/linux/socket_unix_unbound_abstract.cc @@ -90,9 +90,7 @@ TEST_P(UnboundAbstractUnixSocketPairTest, ListenZeroBacklog) { { // Set the FD to O_NONBLOCK. int opts; - int orig_opts; ASSERT_THAT(opts = fcntl(sockets2->first_fd(), F_GETFL), SyscallSucceeds()); - orig_opts = opts; opts |= O_NONBLOCK; ASSERT_THAT(fcntl(sockets2->first_fd(), F_SETFL, opts), SyscallSucceeds()); @@ -104,10 +102,8 @@ TEST_P(UnboundAbstractUnixSocketPairTest, ListenZeroBacklog) { { // Set the FD to O_NONBLOCK. int opts; - int orig_opts; ASSERT_THAT(opts = fcntl(sockets2->second_fd(), F_GETFL), SyscallSucceeds()); - orig_opts = opts; opts |= O_NONBLOCK; ASSERT_THAT(fcntl(sockets2->second_fd(), F_SETFL, opts), SyscallSucceeds()); diff --git a/test/syscalls/linux/udp_socket.cc b/test/syscalls/linux/udp_socket.cc index a870efa53..7fd237eb2 100644 --- a/test/syscalls/linux/udp_socket.cc +++ b/test/syscalls/linux/udp_socket.cc @@ -864,12 +864,7 @@ TEST_P(UdpSocketTest, RecvErrorConnRefused) { // Check the contents of msg. EXPECT_EQ(memcmp(got, buf, sizeof(buf)), 0); // iovec check - // TODO(b/176251997): The next check fails on the gvisor platform due to the - // kernel bug. - if (!IsRunningWithHostinet() || GvisorPlatform() == Platform::kPtrace || - GvisorPlatform() == Platform::kKVM || - GvisorPlatform() == Platform::kNative) - EXPECT_NE(msg.msg_flags & MSG_ERRQUEUE, 0); + EXPECT_NE(msg.msg_flags & MSG_ERRQUEUE, 0); EXPECT_EQ(memcmp(&remote, bind_addr_, addrlen_), 0); // Check the contents of the control message.