Merge "Fix standalone windows build" am: 7b8ff24c28
Original change: https://android-review.googlesource.com/c/platform/external/perfetto/+/1913325
Change-Id: Ib3332a18cb6b68a74de59c1860a902c30a5a2132
diff --git a/src/base/file_utils.cc b/src/base/file_utils.cc
index 7ccbad4..8f998dd 100644
--- a/src/base/file_utils.cc
+++ b/src/base/file_utils.cc
@@ -237,15 +237,14 @@
if (glob_path.length() + 1 > MAX_PATH)
return base::ErrStatus("Directory path %s is too long", dir_path.c_str());
WIN32_FIND_DATAA ffd;
- // We do not use a ScopedResource for the HANDLE from FindFirstFile because
- // the invalid value INVALID_HANDLE_VALUE is not a constexpr under some
- // compile configurations, and thus cannot be used as a template argument.
- HANDLE hFind = FindFirstFileA(glob_path.c_str(), &ffd);
- if (hFind == INVALID_HANDLE_VALUE) {
+
+ base::ScopedResource<HANDLE, FindClose, nullptr, false,
+ base::PlatformHandleChecker>
+ hFind(FindFirstFileA(glob_path.c_str(), &ffd));
+ if (!hFind) {
// For empty directories, there should be at least one entry '.'.
// If FindFirstFileA returns INVALID_HANDLE_VALUE, this means directory
// couldn't be accessed.
- FindClose(hFind);
return base::ErrStatus("Failed to open directory %s", cur_dir.c_str());
}
do {
@@ -254,13 +253,12 @@
if (ffd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) {
std::string subdir_path = cur_dir + ffd.cFileName + '/';
dir_queue.push_back(subdir_path);
- } else if (ffd.dwFileAttributes & FILE_ATTRIBUTE_NORMAL) {
+ } else {
const std::string full_path = cur_dir + ffd.cFileName;
PERFETTO_CHECK(full_path.length() > root_dir_path.length());
output.push_back(full_path.substr(root_dir_path.length()));
}
- } while (FindNextFileA(hFind, &ffd));
- FindClose(hFind);
+ } while (FindNextFileA(*hFind, &ffd));
#else
ScopedDir dir = ScopedDir(opendir(cur_dir.c_str()));
if (!dir) {
diff --git a/src/base/http/http_server.cc b/src/base/http/http_server.cc
index 17a1652..1a90339 100644
--- a/src/base/http/http_server.cc
+++ b/src/base/http/http_server.cc
@@ -464,11 +464,12 @@
append("HTTP/1.1 ");
append(http_code);
append("\r\n");
- for (const char* hdr : headers) {
- if (strlen(hdr) == 0)
+ for (const char* hdr_cstr : headers) {
+ StringView hdr = (hdr_cstr);
+ if (hdr.empty())
continue;
- has_connection_header |= strncasecmp(hdr, "connection:", 11) == 0;
- append(hdr);
+ has_connection_header |= hdr.substr(0, 11).CaseInsensitiveEq("connection:");
+ append(hdr_cstr);
append("\r\n");
}
content_len_actual_ = 0;
diff --git a/src/base/http/http_server_unittest.cc b/src/base/http/http_server_unittest.cc
index 9726ab6..be65198 100644
--- a/src/base/http/http_server_unittest.cc
+++ b/src/base/http/http_server_unittest.cc
@@ -66,16 +66,17 @@
auto checkpoint = task_runner_->CreateCheckpoint(checkpoint_name);
std::string rxbuf;
sock.SetBlocking(false);
- task_runner_->AddFileDescriptorWatch(sock.fd(), [&] {
+ task_runner_->AddFileDescriptorWatch(sock.watch_handle(), [&] {
char buf[1024]{};
auto rsize = PERFETTO_EINTR(sock.Receive(buf, sizeof(buf)));
- ASSERT_GE(rsize, 0);
+ if (rsize < 0)
+ return;
rxbuf.append(buf, static_cast<size_t>(rsize));
if (rsize == 0 || (min_bytes && rxbuf.length() >= min_bytes))
checkpoint();
});
task_runner_->RunUntilCheckpoint(checkpoint_name);
- task_runner_->RemoveFileDescriptorWatch(sock.fd());
+ task_runner_->RemoveFileDescriptorWatch(sock.watch_handle());
return rxbuf;
}
@@ -289,7 +290,9 @@
srv_.AddAllowedOrigin("notallowed.com");
HttpCli cli(&task_runner_);
- EXPECT_CALL(handler_, OnHttpConnectionClosed(_));
+ auto close_checkpoint = task_runner_.CreateCheckpoint("close");
+ EXPECT_CALL(handler_, OnHttpConnectionClosed(_))
+ .WillOnce(InvokeWithoutArgs(close_checkpoint));
EXPECT_CALL(handler_, OnHttpRequest(_))
.WillOnce(Invoke([&](const HttpRequest& req) {
EXPECT_EQ(req.origin.ToStdString(), "http://notallowed.com");
@@ -314,6 +317,7 @@
EXPECT_EQ(cli.Recv(expected_resp.size()), expected_resp);
cli.sock.Shutdown();
+ task_runner_.RunUntilCheckpoint("close");
}
} // namespace
diff --git a/src/base/http/sha1.cc b/src/base/http/sha1.cc
index da3f753..f3e1e5d 100644
--- a/src/base/http/sha1.cc
+++ b/src/base/http/sha1.cc
@@ -30,7 +30,7 @@
#if defined(__GNUC__)
return __builtin_bswap32(x);
#elif defined(_MSC_VER)
- return _byteswap_ulong(val);
+ return _byteswap_ulong(x);
#else
return (((x & 0xff000000u) >> 24) | ((x & 0x00ff0000u) >> 8) |
((x & 0x0000ff00u) << 8) | ((x & 0x000000ffu) << 24));
diff --git a/src/base/unix_socket_unittest.cc b/src/base/unix_socket_unittest.cc
index c838663..45c09d5 100644
--- a/src/base/unix_socket_unittest.cc
+++ b/src/base/unix_socket_unittest.cc
@@ -332,53 +332,6 @@
tx_thread.join();
}
-// Regression test for b/193234818. SO_SNDTIMEO is unreliable on most systems.
-// It doesn't guarantee that the whole send() call blocks for at most X, as the
-// kernel rearms the timeout if the send buffers frees up and allows a partial
-// send. This test reproduces the issue 100% on Mac. Unfortunately on Linux the
-// repro seem to happen only when a suspend happens in the middle.
-TEST_F(UnixSocketTest, BlockingSendTimeout) {
- TestTaskRunner ttr;
- UnixSocketRaw send_sock;
- UnixSocketRaw recv_sock;
- std::tie(send_sock, recv_sock) =
- UnixSocketRaw::CreatePairPosix(kTestSocket.family(), SockType::kStream);
-
- auto blocking_send_done = ttr.CreateCheckpoint("blocking_send_done");
-
- std::thread tx_thread([&] {
- // Fill the tx buffer in non-blocking mode.
- send_sock.SetBlocking(false);
- char buf[1024 * 16]{};
- while (send_sock.Send(buf, sizeof(buf)) > 0) {
- }
-
- // Then do a blocking send. It should return a partial value within the tx
- // timeout.
- send_sock.SetBlocking(true);
- send_sock.SetTxTimeout(10);
- ASSERT_LT(send_sock.Send(buf, sizeof(buf)),
- static_cast<ssize_t>(sizeof(buf)));
- ttr.PostTask(blocking_send_done);
- });
-
- // This task needs to be slow enough so that doesn't unblock the send, but
- // fast enough so that within a blocking cycle, the send re-attempts and
- // re-arms the timeout.
- PeriodicTask read_slowly_task(&ttr);
- PeriodicTask::Args args;
- args.period_ms = 1; // Read 1 byte every ms (1 KiB/s).
- args.task = [&] {
- char rxbuf[1]{};
- recv_sock.Receive(rxbuf, sizeof(rxbuf));
- };
- read_slowly_task.Start(args);
-
- ttr.RunUntilCheckpoint("blocking_send_done");
- read_slowly_task.Reset();
- tx_thread.join();
-}
-
// Regression test for b/76155349 . If the receiver end disconnects while the
// sender is in the middle of a large send(), the socket should gracefully give
// up (i.e. Shutdown()) but not crash.
@@ -939,6 +892,53 @@
ASSERT_EQ(hdr.msg_iov, nullptr);
ASSERT_EQ(memcmp(&send_buf[0], &recv_buf[0], send_buf.size()), 0);
}
+
+// Regression test for b/193234818. SO_SNDTIMEO is unreliable on most systems.
+// It doesn't guarantee that the whole send() call blocks for at most X, as the
+// kernel rearms the timeout if the send buffers frees up and allows a partial
+// send. This test reproduces the issue 100% on Mac. Unfortunately on Linux the
+// repro seem to happen only when a suspend happens in the middle.
+TEST_F(UnixSocketTest, BlockingSendTimeout) {
+ TestTaskRunner ttr;
+ UnixSocketRaw send_sock;
+ UnixSocketRaw recv_sock;
+ std::tie(send_sock, recv_sock) =
+ UnixSocketRaw::CreatePairPosix(kTestSocket.family(), SockType::kStream);
+
+ auto blocking_send_done = ttr.CreateCheckpoint("blocking_send_done");
+
+ std::thread tx_thread([&] {
+ // Fill the tx buffer in non-blocking mode.
+ send_sock.SetBlocking(false);
+ char buf[1024 * 16]{};
+ while (send_sock.Send(buf, sizeof(buf)) > 0) {
+ }
+
+ // Then do a blocking send. It should return a partial value within the tx
+ // timeout.
+ send_sock.SetBlocking(true);
+ send_sock.SetTxTimeout(10);
+ ASSERT_LT(send_sock.Send(buf, sizeof(buf)),
+ static_cast<ssize_t>(sizeof(buf)));
+ ttr.PostTask(blocking_send_done);
+ });
+
+ // This task needs to be slow enough so that doesn't unblock the send, but
+ // fast enough so that within a blocking cycle, the send re-attempts and
+ // re-arms the timeout.
+ PeriodicTask read_slowly_task(&ttr);
+ PeriodicTask::Args args;
+ args.period_ms = 1; // Read 1 byte every ms (1 KiB/s).
+ args.task = [&] {
+ char rxbuf[1]{};
+ recv_sock.Receive(rxbuf, sizeof(rxbuf));
+ };
+ read_slowly_task.Start(args);
+
+ ttr.RunUntilCheckpoint("blocking_send_done");
+ read_slowly_task.Reset();
+ tx_thread.join();
+}
#endif // !OS_WIN
} // namespace
diff --git a/src/trace_processor/timestamped_trace_piece.h b/src/trace_processor/timestamped_trace_piece.h
index ca61546..1b1a0f6 100644
--- a/src/trace_processor/timestamped_trace_piece.h
+++ b/src/trace_processor/timestamped_trace_piece.h
@@ -77,9 +77,19 @@
std::array<double, kMaxNumExtraCounters> extra_counter_values = {};
};
+// On Windows std::aligned_storage was broken before VS 2017 15.8 and the
+// compiler (even clang-cl) requires -D_ENABLE_EXTENDED_ALIGNED_STORAGE. Given
+// the alignment here is purely a performance enhancment with no other
+// functional requirement, disable it on Win.
+#if !PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
+#define PERFETTO_TTS_ALIGNMENT alignas(64)
+#else
+#define PERFETTO_TTS_ALIGNMENT
+#endif
+
// A TimestampedTracePiece is (usually a reference to) a piece of a trace that
// is sorted by TraceSorter.
-struct alignas(64) TimestampedTracePiece {
+struct PERFETTO_TTS_ALIGNMENT TimestampedTracePiece {
enum class Type {
kInvalid = 0,
kFtraceEvent,
diff --git a/src/tracing/test/api_test_support.cc b/src/tracing/test/api_test_support.cc
index 0142d28..a03135d 100644
--- a/src/tracing/test/api_test_support.cc
+++ b/src/tracing/test/api_test_support.cc
@@ -27,6 +27,10 @@
#include "test/test_helper.h"
#endif
+#if PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
+#include <Windows.h>
+#endif
+
namespace perfetto {
namespace test {
diff --git a/tools/test_data b/tools/test_data
index cf8ae14..7dd8975 100755
--- a/tools/test_data
+++ b/tools/test_data
@@ -141,7 +141,7 @@
subprocess.check_call(cmd)
with open(fs.path + SUFFIX + '.swp', 'w') as f:
f.write(fs.actual_digest)
- os.rename(fs.path + SUFFIX + '.swp', fs.path + SUFFIX)
+ os.replace(fs.path + SUFFIX + '.swp', fs.path + SUFFIX)
return fs
map_concurrently(upload_one_file, files_to_upload)
@@ -194,7 +194,7 @@
if digest != fs.expected_digest:
raise Exception('Mismatching digest for %s. expected=%s, actual=%s' %
(uri, fs.expected_digest, digest))
- os.rename(tmp_path, fs.path)
+ os.replace(tmp_path, fs.path)
return fs
map_concurrently(download_one_file, files_to_download)