From 155210680a73593ee9bc59f5eda5e0352e4eeacc Mon Sep 17 00:00:00 2001 From: Dave Allison Date: Mon, 31 Aug 2026 13:13:37 -0700 Subject: [PATCH] Fix strict compiler warnings --- .bazelrc | 3 + BUILD.bazel | 12 ++- MODULE.bazel | 5 +- MODULE.bazel.lock | 10 +- co/BUILD.bazel | 15 --- co/bitset.h | 42 ++++++-- co/coroutine.cc | 90 +++++++++++++---- co/coroutine.h | 49 ++++----- co/coroutine_cpp20.cc | 22 +++- co/costress.cc | 6 +- co/test_cpp20.cc | 226 +++++++++++++++++++++--------------------- http_client/main.cc | 64 ++++++++---- http_server/main.cc | 58 ++++++----- 13 files changed, 359 insertions(+), 243 deletions(-) diff --git a/.bazelrc b/.bazelrc index e60c4ad..46c1c08 100644 --- a/.bazelrc +++ b/.bazelrc @@ -1,6 +1,9 @@ # For all builds, use C++17 build --cxxopt="-std=c++17" +# Strict diagnostics for this repository's C++ sources. +build:strict --per_file_copt=(co|http_client|http_server)/.*@-Wall,-Wextra,-Wpedantic,-Wconversion,-Wsign-conversion,-Wshadow,-Wnon-virtual-dtor,-Wold-style-cast,-Wcast-align,-Woverloaded-virtual,-Wnull-dereference,-Wdouble-promotion,-Wformat=2,-Wimplicit-fallthrough,-Wundef,-Wextra-semi,-Wcast-qual,-Wmissing-declarations,-Wheader-hygiene,-Wthread-safety,-Wcomma,-Wrange-loop-analysis,-Wdeprecated,-Werror,-Wno-nullability-extension,-Wno-gcc-compat,-Wno-unknown-warning-option + # For Apple Silicon build:apple_silicon --cpu=darwin_arm64 build:apple_silicon --features=oso_prefix_is_pwd diff --git a/BUILD.bazel b/BUILD.bazel index 34a6e36..1d39910 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -2,17 +2,23 @@ package(default_visibility = ["//visibility:public"]) config_setting( name = "macos_arm64", - values = {"cpu": "darwin_arm64"}, + constraint_values = [ + "@platforms//cpu:arm64", + "@platforms//os:macos", + ], ) config_setting( name = "macos_default", - values = {"cpu": "darwin"}, + constraint_values = ["@platforms//os:macos"], ) config_setting( name = "macos_x86_64", - values = {"cpu": "darwin_x86_64"}, + constraint_values = [ + "@platforms//cpu:x86_64", + "@platforms//os:macos", + ], ) alias( diff --git a/MODULE.bazel b/MODULE.bazel index f9bc54e..a5e7453 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -1,9 +1,10 @@ module( name = "coroutines", - version = "3.3.2", + version = "3.3.3", ) bazel_dep(name = "bazel_skylib", version = "1.9.0") +bazel_dep(name = "platforms", version = "1.0.0") bazel_dep(name = "abseil-cpp", version = "20250814.1") bazel_dep(name = "googletest", version = "1.17.0.bcr.2") -bazel_dep(name = "rules_cc", version = "0.2.16") +bazel_dep(name = "rules_cc", version = "0.2.17") diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index ad95fc0..b78cfe5 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -1,5 +1,5 @@ { - "lockFileVersion": 26, + "lockFileVersion": 28, "registryFileHashes": { "https://bcr.bazel.build/bazel_registry.json": "8a28e4aff06ee60aed2a8c281907fb8bcbf3b753c91fb5a5c57da3215d5b3497", "https://bcr.bazel.build/modules/abseil-cpp/20210324.2/MODULE.bazel": "7cd0312e064fde87c8d1cd79ba06c876bd23630c83466e9500321be55c96ace2", @@ -121,7 +121,6 @@ "https://bcr.bazel.build/modules/rules_cc/0.1.5/MODULE.bazel": "88dfc9361e8b5ae1008ac38f7cdfd45ad738e4fa676a3ad67d19204f045a1fd8", "https://bcr.bazel.build/modules/rules_cc/0.2.0/MODULE.bazel": "b5c17f90458caae90d2ccd114c81970062946f49f355610ed89bebf954f5783c", "https://bcr.bazel.build/modules/rules_cc/0.2.13/MODULE.bazel": "eecdd666eda6be16a8d9dc15e44b5c75133405e820f620a234acc4b1fdc5aa37", - "https://bcr.bazel.build/modules/rules_cc/0.2.16/MODULE.bazel": "9242fa89f950c6ef7702801ab53922e99c69b02310c39fb6e62b2bd30df2a1d4", "https://bcr.bazel.build/modules/rules_cc/0.2.17/MODULE.bazel": "1849602c86cb60da8613d2de887f9566a6d354a6df6d7009f9d04a14402f9a84", "https://bcr.bazel.build/modules/rules_cc/0.2.17/source.json": "3832f45d145354049137c0090df04629d9c2b5493dc5c2bf46f1834040133a07", "https://bcr.bazel.build/modules/rules_cc/0.2.8/MODULE.bazel": "f1df20f0bf22c28192a794f29b501ee2018fa37a3862a1a2132ae2940a23a642", @@ -203,7 +202,7 @@ "moduleExtensions": { "@@rules_kotlin+//src/main/starlark/core/repositories:bzlmod_setup.bzl%rules_kotlin_extensions": { "general": { - "bzlTransitiveDigest": "Ga4z8lQy1YQ5rAMy+dOl0dqcCEBnYNCXku8x3YQmDZI=", + "bzlTransitiveDigest": "+Kp6j204mBZ3mxlIDDR0gBoP45BZ4jYRhRAcB8sU0qc=", "usagesDigest": "QI2z8ZUR+mqtbwsf2fLqYdJAkPOHdOV+tF2yVAUgRzw=", "recordedInputs": [ "REPO_MAPPING:rules_kotlin+,bazel_tools bazel_tools" @@ -260,7 +259,7 @@ }, "@@rules_python+//python/extensions:config.bzl%config": { "general": { - "bzlTransitiveDigest": "iibnRYgg8LpcfmH7EAnVwYePC3jsVaJ6Id8XxUjSZps=", + "bzlTransitiveDigest": "dzD8Q2YmrP3fz8saWLHPmlwPLO91ImtTmP/c9JKTStM=", "usagesDigest": "ZVSXMAGpD+xzVNPuvF1IoLBkty7TROO0+akMapt1pAg=", "recordedInputs": [ "REPO_MAPPING:rules_python+,bazel_tools bazel_tools", @@ -457,5 +456,6 @@ } } }, - "facts": {} + "facts": {}, + "factsVersions": {} } diff --git a/co/BUILD.bazel b/co/BUILD.bazel index ad982ad..94328f4 100644 --- a/co/BUILD.bazel +++ b/co/BUILD.bazel @@ -2,21 +2,6 @@ package(default_visibility = ["//visibility:public"]) load("@rules_cc//cc:defs.bzl", "cc_library", "cc_binary", "cc_test") -config_setting( - name = "macos_arm64", - values = {"cpu": "darwin_arm64"}, -) - -config_setting( - name = "macos_default", - values = {"cpu": "darwin"}, -) - -config_setting( - name = "macos_x86_64", - values = {"cpu": "darwin_x86_64"}, -) - cc_library( name = "co", srcs = [ diff --git a/co/bitset.h b/co/bitset.h index f3f1a3a..6aaf2e3 100644 --- a/co/bitset.h +++ b/co/bitset.h @@ -10,6 +10,8 @@ #include #include #include +#include +#include #include namespace co { @@ -23,7 +25,13 @@ class BitSet { BitSet() = default; BitSet(int num_bits) { Resize(num_bits); } - void Resize(int num_bits) { bits_.resize(BitsToWords(num_bits)); } + void Resize(int num_bits) { + if (num_bits <= 0) { + bits_.clear(); + return; + } + bits_.resize(BitsToWords(static_cast(num_bits))); + } // Allocate the first free bit. std::uint32_t Allocate(); @@ -46,7 +54,14 @@ class BitSet { } } - int SizeInBits() const { return bits_.size() * 64; } + int SizeInBits() const { + constexpr size_t kBitsPerWord = 64; + if (bits_.size() > + static_cast(std::numeric_limits::max()) / kBitsPerWord) { + return std::numeric_limits::max(); + } + return static_cast(bits_.size() * kBitsPerWord); + } private: // Note the use of explicit long long type here because @@ -58,18 +73,29 @@ class BitSet { inline std::uint32_t BitSet::Allocate() { size_t start = 0; for (;;) { - for (std::uint32_t i = start; i < bits_.size(); i++) { - size_t bit = static_cast(ffsll(~bits_[i])); - if (bit != 0) { - bits_[i] |= (1LL << (bit - 1)); - return i * 64 + (bit - 1); + for (size_t i = start; i < bits_.size(); i++) { + const int bit_index = ffsll(~bits_[i]); + if (bit_index == 0) { + continue; + } + const size_t bit = static_cast(bit_index); + const size_t allocated = i * 64 + (bit - 1); + if (allocated > std::numeric_limits::max()) { + throw std::overflow_error("BitSet index exceeds uint32_t"); } + bits_[i] |= (1ULL << (bit - 1)); + return static_cast(allocated); } // Expand bit set and allocate again. There's no point in // searching the whole bitset again because we know it won't // have any zero bits in it, so start at the newly added // word of zeroes. - start = static_cast(bits_.size()); + constexpr size_t kMaxWords = + static_cast(std::numeric_limits::max()) / 64 + 1; + if (bits_.size() >= kMaxWords) { + throw std::overflow_error("BitSet exhausted uint32_t index space"); + } + start = bits_.size(); bits_.push_back(0); } } diff --git a/co/coroutine.cc b/co/coroutine.cc index 8ff6da7..866755f 100644 --- a/co/coroutine.cc +++ b/co/coroutine.cc @@ -13,6 +13,7 @@ #include #include #include +#include #include "bitset.h" @@ -106,6 +107,33 @@ constexpr bool kCoDebug = false; namespace co { +namespace { + +short PollEventsFromMask(uint32_t event_mask) { + if (event_mask > static_cast(std::numeric_limits::max())) { + abort(); + } + return static_cast(event_mask); +} + +bool PollNfdsFromSize(size_t count, nfds_t *nfds) { + if (count > static_cast(std::numeric_limits::max())) { + return false; + } + *nfds = static_cast(count); + return true; +} + +bool FdToBitIndex(int fd, std::uint32_t *bit_index) { + if (fd < 0) { + return false; + } + *bit_index = static_cast(fd); + return true; +} + +} // namespace + struct AbortException {}; #if CO_EVENT_MODE == CO_EVENT_PIPE @@ -280,30 +308,31 @@ asm( #endif // clang-format on -Coroutine::Coroutine(CoroutineScheduler &scheduler, CoroutineFunction functor, +Coroutine::Coroutine(CoroutineScheduler &coroutine_scheduler, + CoroutineFunction functor, std::string name, int interrupt_fd, bool autostart, size_t stack_size, void *user_data) - : Coroutine(scheduler, + : Coroutine(coroutine_scheduler, [functor = std::move(functor)](const Coroutine &c) { functor(const_cast(&c)); }, std::move(name), interrupt_fd, autostart, stack_size, user_data) {} -Coroutine::Coroutine(CoroutineScheduler &scheduler, +Coroutine::Coroutine(CoroutineScheduler &coroutine_scheduler, std::function functor, std::string name, int interrupt_fd, bool autostart, size_t stack_size, void *user_data) : Coroutine( - scheduler, + coroutine_scheduler, [functor = std::move(functor)](const Coroutine &) { functor(); }, std::move(name), interrupt_fd, autostart, stack_size, user_data) {} -Coroutine::Coroutine(CoroutineScheduler &scheduler, +Coroutine::Coroutine(CoroutineScheduler &coroutine_scheduler, CoroutineFunctionRef functor, std::string name, int interrupt_fd, bool autostart, size_t stack_size, void *user_data) - : scheduler_(scheduler), function_(std::move(functor)), + : scheduler_(coroutine_scheduler), function_(std::move(functor)), interrupt_fd_(dup(interrupt_fd)), user_data_(user_data) { id_ = scheduler_.AllocateId(); if (name.empty()) { @@ -647,13 +676,17 @@ int Coroutine::Poll(const std::vector &fds, short event_mask) const { std::vector pfds; pfds.reserve(fds.size() + 1); for (auto &fd : fds) { - pfds.push_back({.fd = fd, .events = short(event_mask), .revents = 0}); + pfds.push_back({.fd = fd, .events = event_mask, .revents = 0}); } if (interrupt_fd_ != -1) { struct pollfd ifd = {.fd = interrupt_fd_, .events = POLLIN, .revents = 0}; pfds.push_back(ifd); } - int ret = ::poll(pfds.data(), pfds.size(), 0); + nfds_t nfds = 0; + if (!PollNfdsFromSize(pfds.size(), &nfds)) { + return -1; + } + int ret = ::poll(pfds.data(), nfds, 0); if (ret <= 0) { return -1; } @@ -673,7 +706,11 @@ int Coroutine::Poll(const std::vector &fds) const { struct pollfd ifd = {.fd = interrupt_fd_, .events = POLLIN, .revents = 0}; pfds.push_back(ifd); } - int ret = ::poll(pfds.data(), pfds.size(), 0); + nfds_t nfds = 0; + if (!PollNfdsFromSize(pfds.size(), &nfds)) { + return -1; + } + int ret = ::poll(pfds.data(), nfds, 0); if (ret <= 0) { return -1; } @@ -692,7 +729,7 @@ int Coroutine::Wait(int fd, uint32_t event_mask, uint64_t timeout_ns) const { wait_fds_.push_back(YieldedCoroutine(this, interrupt_fd_, EPOLLIN)); } #else - struct pollfd pfd = {.fd = fd, .events = short(event_mask)}; + struct pollfd pfd = {.fd = fd, .events = PollEventsFromMask(event_mask)}; wait_fds_.push_back(pfd); if (interrupt_fd_ != -1) { struct pollfd ifd = {.fd = interrupt_fd_, .events = POLLIN}; @@ -721,7 +758,7 @@ int Coroutine::Wait(const std::vector &fds, uint32_t event_mask, } #else for (auto &fd : fds) { - wait_fds_.push_back({.fd = fd, .events = short(event_mask)}); + wait_fds_.push_back({.fd = fd, .events = PollEventsFromMask(event_mask)}); } if (interrupt_fd_ != -1) { struct pollfd ifd = {.fd = interrupt_fd_, .events = POLLIN}; @@ -741,7 +778,7 @@ int Coroutine::Wait(const std::vector &fds, uint32_t event_mask, int Coroutine::PollAndWait(int fd, uint32_t event_mask, uint64_t timeout_ns) const { - int n = Poll({fd}, event_mask); + int n = Poll({fd}, PollEventsFromMask(event_mask)); if (n != -1) { return n; } @@ -750,7 +787,7 @@ int Coroutine::PollAndWait(int fd, uint32_t event_mask, int Coroutine::PollAndWait(const std::vector &fds, uint32_t event_mask, uint64_t timeout_ns) const { - int n = Poll(fds, event_mask); + int n = Poll(fds, PollEventsFromMask(event_mask)); if (n != -1) { return n; } @@ -780,7 +817,8 @@ int Coroutine::PollAndWait(const std::vector &fds, std::vector pfds; pfds.reserve(fds.size()); for (auto &fd : fds) { - pfds.push_back({.fd = fd.fd, .events = short(fd.events), .revents = 0}); + pfds.push_back( + {.fd = fd.fd, .events = PollEventsFromMask(fd.events), .revents = 0}); } if (interrupt_fd_ != -1) { struct pollfd ifd = {.fd = interrupt_fd_, .events = POLLIN, .revents = 0}; @@ -1354,7 +1392,9 @@ void CoroutineScheduler::Run() { // converting the epoll_event to a YieldedCoroutine. events.clear(); - events.reserve(num_ready); + if (num_ready > 0) { + events.reserve(static_cast(num_ready)); + } for (int i = 0; i < num_ready; i++) { struct epoll_event &event = epoll_events[i]; if (event.data.fd > max_fd) { @@ -1392,8 +1432,11 @@ void CoroutineScheduler::Run() { #else // Poll mode. BuildPollFds(&poll_state_); - int num_ready = - ::poll(poll_state_.pollfds.data(), poll_state_.pollfds.size(), -1); + nfds_t nfds = 0; + if (!PollNfdsFromSize(poll_state_.pollfds.size(), &nfds)) { + return; + } + int num_ready = ::poll(poll_state_.pollfds.data(), nfds, -1); if (num_ready <= 0) { continue; } @@ -1402,7 +1445,9 @@ void CoroutineScheduler::Run() { } // Copy all triggered pollfds into the events vector. events.clear(); - events.reserve(num_ready); + if (num_ready > 0) { + events.reserve(static_cast(num_ready)); + } constexpr size_t kNumReservedFds = 2; for (size_t i = 0; i < poll_state_.pollfds.size(); i++) { if (poll_state_.pollfds[i].fd > max_fd) { @@ -1465,7 +1510,7 @@ void CoroutineScheduler::Run() { if (index >= num_ready) { break; } - YieldedCoroutine *c = &events[index]; + YieldedCoroutine *c = &events[static_cast(index)]; index++; if (c->fd == interrupt_fd_.poll_fd) { interrupt_fd_.Clear(); @@ -1482,7 +1527,8 @@ void CoroutineScheduler::Run() { } tick_count_++; - if (processed_fds.Contains(c->fd)) { + std::uint32_t fd_bit = 0; + if (FdToBitIndex(c->fd, &fd_bit) && processed_fds.Contains(fd_bit)) { // Since we can have more than one coroutine waiting for an fd we need // to check that the fd is still ready to prevent blocking. We only do // this if the fd is in blocking mode. @@ -1501,7 +1547,9 @@ void CoroutineScheduler::Run() { } } } - processed_fds.Set(c->fd); + if (FdToBitIndex(c->fd, &fd_bit)) { + processed_fds.Set(fd_bit); + } // Clear the event for the corouutine since we will be resuming // it. diff --git a/co/coroutine.h b/co/coroutine.h index 5e2a4f3..bbd5d0a 100644 --- a/co/coroutine.h +++ b/co/coroutine.h @@ -190,10 +190,12 @@ using Context = co::CoroutineContext; #endif #include +#include #include #include #include #include +#include #include #include #include @@ -231,6 +233,25 @@ void __tsan_set_fiber_name(void *fiber, const char *name); namespace co { +namespace detail { + +template +inline uint64_t ChronoToNanoseconds(std::chrono::duration duration) { + using FloatingNanoseconds = std::chrono::duration; + const long double count = FloatingNanoseconds(duration).count(); + if (!(count > 0)) { + return 0; + } + const long double maximum = + static_cast(std::numeric_limits::max()); + if (count >= maximum) { + return std::numeric_limits::max(); + } + return static_cast(count); +} + +} // namespace detail + class CoroutineScheduler; class Coroutine; template class Generator; @@ -472,40 +493,24 @@ class Coroutine { template int Wait(const T &fd, uint32_t events, std::chrono::duration duration) const { - return Wait( - fd, events, - std::chrono::duration_cast>( - duration) - .count()); + return Wait(fd, events, detail::ChronoToNanoseconds(duration)); } template int Wait(const T &fd, std::chrono::duration duration) const { - return Wait( - fd, POLLIN, - std::chrono::duration_cast>( - duration) - .count()); + return Wait(fd, POLLIN, detail::ChronoToNanoseconds(duration)); } template int PollAndWait(const T &fd, uint32_t events, std::chrono::duration duration) const { - return PollAndWait( - fd, events, - std::chrono::duration_cast>( - duration) - .count()); + return PollAndWait(fd, events, detail::ChronoToNanoseconds(duration)); } template int PollAndWait(const T &fd, std::chrono::duration duration) const { - return PollAndWait( - fd, POLLIN, - std::chrono::duration_cast>( - duration) - .count()); + return PollAndWait(fd, POLLIN, detail::ChronoToNanoseconds(duration)); } // Note this can cause memory leaks as destructors in the coroutine function @@ -525,9 +530,7 @@ class Coroutine { template void Sleep(std::chrono::duration duration) const { - Nanosleep(std::chrono::duration_cast>( - duration) - .count()); + Nanosleep(detail::ChronoToNanoseconds(duration)); } // Abort the coroutine. It will cause the current wait or sleep to throw an diff --git a/co/coroutine_cpp20.cc b/co/coroutine_cpp20.cc index 0e26b8c..6e5b346 100644 --- a/co/coroutine_cpp20.cc +++ b/co/coroutine_cpp20.cc @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -111,18 +112,24 @@ void Scheduler::ScheduleCoroutine(Coroutine* coroutine) { } int Scheduler::PollFd(int fd, uint32_t event_mask) { + if (event_mask > + static_cast(std::numeric_limits::max())) { + return -1; + } struct pollfd pfd; pfd.fd = fd; - pfd.events = event_mask; + pfd.events = static_cast(event_mask); pfd.revents = 0; int ret = poll(&pfd, 1, 0); if (ret <= 0) return -1; + const uint32_t revents = + static_cast(static_cast(pfd.revents)); if (event_mask & POLLIN) { - if (pfd.revents & (POLLIN | POLLERR)) return fd; + if (revents & (POLLIN | POLLERR)) return fd; } else { - if ((pfd.revents & event_mask) || (pfd.revents & POLLERR)) return fd; + if ((revents & event_mask) || (revents & POLLERR)) return fd; } return -1; } @@ -678,10 +685,15 @@ void Scheduler::ProcessEvents() { interrupt_pfd.revents = 0; pfds.push_back(interrupt_pfd); - int ret = poll(pfds.data(), pfds.size(), 0); + if (pfds.size() > + static_cast(std::numeric_limits::max())) { + return; + } + const nfds_t nfds = static_cast(pfds.size()); + int ret = poll(pfds.data(), nfds, 0); if (ret <= 0) { if (waiting_fds_.empty()) return; - ret = poll(pfds.data(), pfds.size(), -1); + ret = poll(pfds.data(), nfds, -1); if (ret <= 0) return; } diff --git a/co/costress.cc b/co/costress.cc index 1733864..6ee7968 100644 --- a/co/costress.cc +++ b/co/costress.cc @@ -16,10 +16,10 @@ void Test(Coroutine *c) { } int main() { - CoroutineScheduler scheduler; + CoroutineScheduler sched; std::vector> coroutines; for (int i = 0; i < 1000; i++) { - coroutines.push_back(std::make_unique(scheduler, Test)); + coroutines.push_back(std::make_unique(sched, Test)); } - scheduler.Run(); + sched.Run(); } diff --git a/co/test_cpp20.cc b/co/test_cpp20.cc index 20ca9bc..501f126 100644 --- a/co/test_cpp20.cc +++ b/co/test_cpp20.cc @@ -13,44 +13,44 @@ namespace co20 { TEST(Cpp20, Basic) { - Scheduler scheduler; + Scheduler sched; bool ran = false; - scheduler.Spawn([&ran](Coroutine& co) -> Task { + sched.Spawn([&ran](Coroutine&) -> Task { ran = true; co_return; }, "test"); - scheduler.Run(); + sched.Run(); EXPECT_TRUE(ran); } TEST(Cpp20, Yield) { - Scheduler scheduler; + Scheduler sched; int count = 0; - scheduler.Spawn([&count](Coroutine& co) -> Task { + sched.Spawn([&count](Coroutine& co) -> Task { count++; co_await co.Yield(); count++; co_return; }, "test"); - scheduler.Run(); + sched.Run(); EXPECT_EQ(2, count); } TEST(Cpp20, Wait) { - Scheduler scheduler; + Scheduler sched; int pipes[2]; ASSERT_EQ(0, pipe(pipes)); std::string result; bool reader_done = false; - scheduler.Spawn([&pipes, &result, &reader_done](Coroutine& co) -> Task { + sched.Spawn([&pipes, &result, &reader_done](Coroutine& co) -> Task { for (;;) { int fd = co_await co.Wait(pipes[0], POLLIN); if (fd != pipes[0]) break; @@ -68,7 +68,7 @@ TEST(Cpp20, Wait) { co_return; }, "reader"); - scheduler.Spawn([&pipes](Coroutine& co) -> Task { + sched.Spawn([&pipes](Coroutine& co) -> Task { for (int i = 0; i < 10; i++) { int fd = co_await co.Wait(pipes[1], POLLOUT); if (fd != pipes[1]) { @@ -84,33 +84,33 @@ TEST(Cpp20, Wait) { co_return; }, "writer"); - scheduler.Run(); + sched.Run(); EXPECT_TRUE(reader_done); EXPECT_EQ("ABCDEFGHIJ", result); } TEST(Cpp20, Sleep) { - Scheduler scheduler; + Scheduler sched; bool slept = false; - scheduler.Spawn([&slept](Coroutine& co) -> Task { + sched.Spawn([&slept](Coroutine& co) -> Task { co_await co.Sleep(1000000); // 1ms slept = true; co_return; }, "sleep_test"); - scheduler.Run(); + sched.Run(); EXPECT_TRUE(slept); } TEST(Cpp20, Loop) { - Scheduler scheduler; + Scheduler sched; // Create 10 coroutines, each yielding 10 times for (int i = 0; i < 10; i++) { - scheduler.Spawn([](Coroutine& co) -> Task { + sched.Spawn([](Coroutine& co) -> Task { for (int j = 0; j < 10; j++) { co_await co.Yield(); } @@ -118,7 +118,7 @@ TEST(Cpp20, Loop) { }, "loop_coroutine_" + std::to_string(i)); } - scheduler.Run(); + sched.Run(); } #if CO_POLL_MODE == CO_POLL_EPOLL @@ -127,7 +127,7 @@ TEST(Cpp20, Loop) { // allow multiple fds to be added to the epoll fd so we keep track of // the coroutines ourselves. TEST(Cpp20, MultipleFd) { - Scheduler scheduler; + Scheduler sched; int pipes[2]; ASSERT_EQ(0, pipe(pipes)); @@ -141,7 +141,7 @@ TEST(Cpp20, MultipleFd) { } state; // This will run first. - scheduler.Spawn([pipes, &state](Coroutine& co) -> Task { + sched.Spawn([pipes, &state](Coroutine& co) -> Task { int fd = co_await co.Wait(pipes[0], POLLIN); if (fd != pipes[0]) { co_return; @@ -153,7 +153,7 @@ TEST(Cpp20, MultipleFd) { }, "foo"); // This will run second. - scheduler.Spawn([pipes, &state](Coroutine& co) -> Task { + sched.Spawn([pipes, &state](Coroutine& co) -> Task { // Waiting on the same fd is supported. int fd = co_await co.Wait(pipes[0], POLLIN); if (fd != pipes[0]) { @@ -168,7 +168,7 @@ TEST(Cpp20, MultipleFd) { }, "bar"); // After c1 and c2 we will run this and it will wake up c1. - scheduler.Spawn([pipes, &state](Coroutine& co) -> Task { + sched.Spawn([pipes, &state](Coroutine& co) -> Task { // This will wake up foo but not bar. char buf = 'x'; state.write_result = ::write(pipes[1], &buf, 1); @@ -178,7 +178,7 @@ TEST(Cpp20, MultipleFd) { co_return; }, "baz"); - scheduler.Run(); + sched.Run(); close(pipes[0]); EXPECT_TRUE(state.foo_woke); @@ -189,14 +189,14 @@ TEST(Cpp20, MultipleFd) { } TEST(Cpp20, AbortYield) { - Scheduler scheduler; + Scheduler sched; struct TestState { bool aborted = false; Coroutine* coroutine_ptr = nullptr; } state; - scheduler.Spawn([&state](Coroutine& co) -> Task { + sched.Spawn([&state](Coroutine& co) -> Task { state.coroutine_ptr = &co; try { for (;;) { @@ -208,7 +208,7 @@ TEST(Cpp20, AbortYield) { co_return; }, "sleeping_coroutine"); - scheduler.Spawn([&state](Coroutine& co) -> Task { + sched.Spawn([&state](Coroutine& co) -> Task { co_await co.Sleep(100000000); // 100ms while (!state.coroutine_ptr) { co_await co.Yield(); @@ -220,12 +220,12 @@ TEST(Cpp20, AbortYield) { co_return; }, "abort_coroutine"); - scheduler.Run(); + sched.Run(); EXPECT_TRUE(state.aborted); } TEST(Cpp20, AbortSingle) { - Scheduler scheduler; + Scheduler sched; int pipes[2]; ASSERT_EQ(0, pipe(pipes)); @@ -235,7 +235,7 @@ TEST(Cpp20, AbortSingle) { Coroutine* coroutine_ptr = nullptr; } state; - scheduler.Spawn([pipes, &state](Coroutine& co) -> Task { + sched.Spawn([pipes, &state](Coroutine& co) -> Task { state.coroutine_ptr = &co; try { for (;;) { @@ -247,7 +247,7 @@ TEST(Cpp20, AbortSingle) { co_return; }, "waiting_coroutine"); - scheduler.Spawn([&state](Coroutine& co) -> Task { + sched.Spawn([&state](Coroutine& co) -> Task { co_await co.Sleep(100000000); // 100ms while (!state.coroutine_ptr) { co_await co.Yield(); @@ -259,7 +259,7 @@ TEST(Cpp20, AbortSingle) { co_return; }, "abort_coroutine"); - scheduler.Run(); + sched.Run(); close(pipes[0]); close(pipes[1]); EXPECT_TRUE(state.aborted); @@ -269,59 +269,59 @@ TEST(Cpp20, AbortSingle) { // --- Tests using the free-function API (co20::self, co20::Yield(), etc.) --- TEST(Cpp20Free, Self) { - Scheduler scheduler; + Scheduler sched; const Coroutine* captured_self = nullptr; - scheduler.Spawn([&captured_self]() -> Task { + sched.Spawn([&captured_self]() -> Task { captured_self = co20::self; co_return; }, "self_test"); - scheduler.Run(); + sched.Run(); ASSERT_NE(nullptr, captured_self); EXPECT_EQ("self_test", captured_self->Name()); } TEST(Cpp20Free, SchedulerAccess) { - Scheduler scheduler; + Scheduler sched; Scheduler* captured_scheduler = nullptr; - scheduler.Spawn([&captured_scheduler]() -> Task { + sched.Spawn([&captured_scheduler]() -> Task { captured_scheduler = co20::scheduler; co_return; }, "scheduler_test"); - scheduler.Run(); + sched.Run(); - EXPECT_EQ(&scheduler, captured_scheduler); + EXPECT_EQ(&sched, captured_scheduler); } TEST(Cpp20Free, Yield) { - Scheduler scheduler; + Scheduler sched; int count = 0; - scheduler.Spawn([&count]() -> Task { + sched.Spawn([&count]() -> Task { count++; co_await co20::Yield(); count++; co_return; }, "test"); - scheduler.Run(); + sched.Run(); EXPECT_EQ(2, count); } TEST(Cpp20Free, Wait) { - Scheduler scheduler; + Scheduler sched; int pipes[2]; ASSERT_EQ(0, pipe(pipes)); std::string result; bool reader_done = false; - scheduler.Spawn([&pipes, &result, &reader_done]() -> Task { + sched.Spawn([&pipes, &result, &reader_done]() -> Task { for (;;) { int fd = co_await co20::Wait(pipes[0], POLLIN); if (fd != pipes[0]) break; @@ -339,7 +339,7 @@ TEST(Cpp20Free, Wait) { co_return; }, "reader"); - scheduler.Spawn([&pipes]() -> Task { + sched.Spawn([&pipes]() -> Task { for (int i = 0; i < 10; i++) { int fd = co_await co20::Wait(pipes[1], POLLOUT); if (fd != pipes[1]) { @@ -354,62 +354,62 @@ TEST(Cpp20Free, Wait) { co_return; }, "writer"); - scheduler.Run(); + sched.Run(); EXPECT_TRUE(reader_done); EXPECT_EQ("ABCDEFGHIJ", result); } TEST(Cpp20Free, Sleep) { - Scheduler scheduler; + Scheduler sched; bool slept = false; - scheduler.Spawn([&slept]() -> Task { + sched.Spawn([&slept]() -> Task { co_await co20::Sleep(1000000ULL); // 1ms slept = true; co_return; }, "sleep_test"); - scheduler.Run(); + sched.Run(); EXPECT_TRUE(slept); } TEST(Cpp20Free, SleepChrono) { - Scheduler scheduler; + Scheduler sched; bool slept = false; - scheduler.Spawn([&slept]() -> Task { + sched.Spawn([&slept]() -> Task { co_await co20::Sleep(std::chrono::milliseconds(1)); slept = true; co_return; }, "sleep_chrono_test"); - scheduler.Run(); + sched.Run(); EXPECT_TRUE(slept); } TEST(Cpp20Free, Millisleep) { - Scheduler scheduler; + Scheduler sched; bool slept = false; - scheduler.Spawn([&slept]() -> Task { + sched.Spawn([&slept]() -> Task { co_await co20::Millisleep(1); slept = true; co_return; }, "millisleep_test"); - scheduler.Run(); + sched.Run(); EXPECT_TRUE(slept); } TEST(Cpp20Free, Loop) { - Scheduler scheduler; + Scheduler sched; for (int i = 0; i < 10; i++) { - scheduler.Spawn([]() -> Task { + sched.Spawn([]() -> Task { for (int j = 0; j < 10; j++) { co_await co20::Yield(); } @@ -417,19 +417,19 @@ TEST(Cpp20Free, Loop) { }, "loop_coroutine_" + std::to_string(i)); } - scheduler.Run(); + sched.Run(); } #if CO_POLL_MODE == CO_POLL_EPOLL TEST(Cpp20Free, AbortWithSelf) { - Scheduler scheduler; + Scheduler sched; struct TestState { bool aborted = false; Coroutine* target = nullptr; } state; - scheduler.Spawn([&state]() -> Task { + sched.Spawn([&state]() -> Task { state.target = co20::self; try { for (;;) { @@ -441,7 +441,7 @@ TEST(Cpp20Free, AbortWithSelf) { co_return; }, "target_coroutine"); - scheduler.Spawn([&state]() -> Task { + sched.Spawn([&state]() -> Task { co_await co20::Sleep(std::chrono::milliseconds(100)); while (!state.target) { co_await co20::Yield(); @@ -453,13 +453,13 @@ TEST(Cpp20Free, AbortWithSelf) { co_return; }, "abort_coroutine"); - scheduler.Run(); + sched.Run(); EXPECT_TRUE(state.aborted); } #endif TEST(Cpp20, InterruptFd) { - Scheduler scheduler; + Scheduler sched; #if defined(__linux__) int efd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC); @@ -478,7 +478,7 @@ TEST(Cpp20, InterruptFd) { bool interrupted = false; int wait_result = -1; - scheduler.Spawn([&data_pipes, &interrupted, &wait_result](Coroutine& co) -> Task { + sched.Spawn([&data_pipes, &interrupted, &wait_result](Coroutine& co) -> Task { int fd = co_await co.Wait(data_pipes[0], POLLIN); wait_result = fd; if (fd == co.GetInterruptFd()) { @@ -487,7 +487,7 @@ TEST(Cpp20, InterruptFd) { co_return; }, "waiting", efd); - scheduler.Spawn([ + sched.Spawn([ #if defined(__linux__) &efd #else @@ -505,7 +505,7 @@ TEST(Cpp20, InterruptFd) { co_return; }, "interrupter"); - scheduler.Run(); + sched.Run(); EXPECT_TRUE(interrupted); EXPECT_NE(wait_result, data_pipes[0]); @@ -521,7 +521,7 @@ TEST(Cpp20, InterruptFd) { } TEST(Cpp20, InterruptFdWithFreeFunction) { - Scheduler scheduler; + Scheduler sched; #if defined(__linux__) int efd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC); @@ -540,7 +540,7 @@ TEST(Cpp20, InterruptFdWithFreeFunction) { bool interrupted = false; int wait_result = -1; - scheduler.Spawn([&data_pipes, &interrupted, &wait_result]() -> Task { + sched.Spawn([&data_pipes, &interrupted, &wait_result]() -> Task { int fd = co_await co20::Wait(data_pipes[0], POLLIN); wait_result = fd; if (fd == co20::self->GetInterruptFd()) { @@ -549,7 +549,7 @@ TEST(Cpp20, InterruptFdWithFreeFunction) { co_return; }, "waiting", efd); - scheduler.Spawn([ + sched.Spawn([ #if defined(__linux__) &efd #else @@ -567,7 +567,7 @@ TEST(Cpp20, InterruptFdWithFreeFunction) { co_return; }, "interrupter"); - scheduler.Run(); + sched.Run(); EXPECT_TRUE(interrupted); EXPECT_NE(wait_result, data_pipes[0]); @@ -583,7 +583,7 @@ TEST(Cpp20, InterruptFdWithFreeFunction) { } TEST(Cpp20, InterruptFdDataFirst) { - Scheduler scheduler; + Scheduler sched; #if defined(__linux__) int efd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC); @@ -602,7 +602,7 @@ TEST(Cpp20, InterruptFdDataFirst) { bool got_data = false; int wait_result = -1; - scheduler.Spawn([&data_pipes, &got_data, &wait_result](Coroutine& co) -> Task { + sched.Spawn([&data_pipes, &got_data, &wait_result](Coroutine& co) -> Task { int fd = co_await co.Wait(data_pipes[0], POLLIN); wait_result = fd; if (fd == data_pipes[0]) { @@ -613,14 +613,14 @@ TEST(Cpp20, InterruptFdDataFirst) { co_return; }, "waiting", efd); - scheduler.Spawn([&data_pipes](Coroutine& co) -> Task { + sched.Spawn([&data_pipes](Coroutine& co) -> Task { co_await co.Yield(); char c = 'D'; (void)write(data_pipes[1], &c, 1); co_return; }, "writer"); - scheduler.Run(); + sched.Run(); EXPECT_TRUE(got_data); EXPECT_EQ(wait_result, data_pipes[0]); @@ -643,15 +643,15 @@ ValueTask AddAsync(Coroutine& co, int a, int b) { } TEST(Cpp20ValueTask, BasicReturn) { - Scheduler scheduler; + Scheduler sched; int result = 0; - scheduler.Spawn([&result](Coroutine& co) -> Task { + sched.Spawn([&result](Coroutine& co) -> Task { result = co_await AddAsync(co, 3, 4); co_return; }, "test"); - scheduler.Run(); + sched.Run(); EXPECT_EQ(7, result); } @@ -663,25 +663,25 @@ ValueTask ReadFromPipeAsync(Coroutine& co, int read_fd) { char buf[64]; ssize_t n = read(read_fd, buf, sizeof(buf)); if (n <= 0) break; - data.append(buf, n); + data.append(buf, static_cast(n)); } co_return data; } TEST(Cpp20ValueTask, WithWait) { - Scheduler scheduler; + Scheduler sched; int pipes[2]; ASSERT_EQ(0, pipe(pipes)); std::string result; - scheduler.Spawn([&pipes, &result](Coroutine& co) -> Task { + sched.Spawn([&pipes, &result](Coroutine& co) -> Task { result = co_await ReadFromPipeAsync(co, pipes[0]); close(pipes[0]); co_return; }, "reader"); - scheduler.Spawn([&pipes](Coroutine& co) -> Task { + sched.Spawn([&pipes](Coroutine& co) -> Task { co_await co.Yield(); const char* msg = "hello"; (void)write(pipes[1], msg, 5); @@ -690,7 +690,7 @@ TEST(Cpp20ValueTask, WithWait) { co_return; }, "writer"); - scheduler.Run(); + sched.Run(); EXPECT_EQ("hello", result); } @@ -706,15 +706,15 @@ ValueTask OuterCompute(Coroutine& co, int x) { } TEST(Cpp20ValueTask, Nested) { - Scheduler scheduler; + Scheduler sched; int result = 0; - scheduler.Spawn([&result](Coroutine& co) -> Task { + sched.Spawn([&result](Coroutine& co) -> Task { result = co_await OuterCompute(co, 5); co_return; }, "test"); - scheduler.Run(); + sched.Run(); EXPECT_EQ(11, result); } @@ -725,17 +725,17 @@ ValueTask IncrementAsync(Coroutine& co, int& counter) { } TEST(Cpp20ValueTask, VoidReturn) { - Scheduler scheduler; + Scheduler sched; int counter = 0; - scheduler.Spawn([&counter](Coroutine& co) -> Task { + sched.Spawn([&counter](Coroutine& co) -> Task { co_await IncrementAsync(co, counter); co_await IncrementAsync(co, counter); co_await IncrementAsync(co, counter); co_return; }, "test"); - scheduler.Run(); + sched.Run(); EXPECT_EQ(3, counter); } @@ -747,13 +747,13 @@ ValueTask WaitAndWrite(Coroutine& co, int write_fd) { } TEST(Cpp20ValueTask, VoidWithWait) { - Scheduler scheduler; + Scheduler sched; int pipes[2]; ASSERT_EQ(0, pipe(pipes)); char read_buf = 0; - scheduler.Spawn([&pipes, &read_buf](Coroutine& co) -> Task { + sched.Spawn([&pipes, &read_buf](Coroutine& co) -> Task { co_await WaitAndWrite(co, pipes[1]); close(pipes[1]); int fd = co_await co.Wait(pipes[0], POLLIN); @@ -763,7 +763,7 @@ TEST(Cpp20ValueTask, VoidWithWait) { co_return; }, "test"); - scheduler.Run(); + sched.Run(); EXPECT_EQ('V', read_buf); } @@ -773,30 +773,30 @@ ValueTask AddWithFreeFunction(int a, int b) { } TEST(Cpp20ValueTask, FreeFunctions) { - Scheduler scheduler; + Scheduler sched; int result = 0; - scheduler.Spawn([&result]() -> Task { + sched.Spawn([&result]() -> Task { result = co_await AddWithFreeFunction(10, 20); co_return; }, "test"); - scheduler.Run(); + sched.Run(); EXPECT_EQ(30, result); } TEST(Cpp20ValueTask, MultipleSequentialCalls) { - Scheduler scheduler; + Scheduler sched; int sum = 0; - scheduler.Spawn([&sum](Coroutine& co) -> Task { + sched.Spawn([&sum](Coroutine& co) -> Task { for (int i = 1; i <= 5; i++) { sum += co_await AddAsync(co, i, 0); } co_return; }, "test"); - scheduler.Run(); + sched.Run(); EXPECT_EQ(15, sum); } @@ -806,13 +806,13 @@ ValueTask CheckPipeReady(Coroutine& co, int read_fd, uint64_t timeout_ns) } TEST(Cpp20ValueTask, BoolReturn) { - Scheduler scheduler; + Scheduler sched; int pipes[2]; ASSERT_EQ(0, pipe(pipes)); bool pipe_ready = false; - scheduler.Spawn([&pipes, &pipe_ready](Coroutine& co) -> Task { + sched.Spawn([&pipes, &pipe_ready](Coroutine& co) -> Task { // Write data first so the pipe is ready. char c = 'x'; (void)write(pipes[1], &c, 1); @@ -823,18 +823,18 @@ TEST(Cpp20ValueTask, BoolReturn) { co_return; }, "test"); - scheduler.Run(); + sched.Run(); EXPECT_TRUE(pipe_ready); } TEST(Cpp20WaitTimeout, DataArrivesBeforeTimeout) { - Scheduler scheduler; + Scheduler sched; int pipes[2]; ASSERT_EQ(0, pipe(pipes)); bool success = false; - scheduler.Spawn([&](Coroutine& co) -> Task { + sched.Spawn([&](Coroutine& co) -> Task { char c = 'x'; (void)write(pipes[1], &c, 1); co_await co.Yield(); @@ -847,18 +847,18 @@ TEST(Cpp20WaitTimeout, DataArrivesBeforeTimeout) { co_return; }, "test"); - scheduler.Run(); + sched.Run(); EXPECT_TRUE(success); } TEST(Cpp20WaitTimeout, TimeoutExpires) { - Scheduler scheduler; + Scheduler sched; int pipes[2]; ASSERT_EQ(0, pipe(pipes)); bool timed_out = false; - scheduler.Spawn([&](Coroutine& co) -> Task { + sched.Spawn([&](Coroutine& co) -> Task { int fd = co_await co.Wait(pipes[0], POLLIN, 50000000ULL); timed_out = (fd != pipes[0]); @@ -867,18 +867,18 @@ TEST(Cpp20WaitTimeout, TimeoutExpires) { co_return; }, "test"); - scheduler.Run(); + sched.Run(); EXPECT_TRUE(timed_out); } TEST(Cpp20WaitTimeout, FreeFunctionTimeout) { - Scheduler scheduler; + Scheduler sched; int pipes[2]; ASSERT_EQ(0, pipe(pipes)); bool timed_out = false; - scheduler.Spawn([&]() -> Task { + sched.Spawn([&]() -> Task { int fd = co_await co20::Wait(pipes[0], POLLIN, 50000000ULL); timed_out = (fd != pipes[0]); @@ -887,18 +887,18 @@ TEST(Cpp20WaitTimeout, FreeFunctionTimeout) { co_return; }, "test"); - scheduler.Run(); + sched.Run(); EXPECT_TRUE(timed_out); } TEST(Cpp20WaitTimeout, ValueTaskWithTimeout) { - Scheduler scheduler; + Scheduler sched; int pipes[2]; ASSERT_EQ(0, pipe(pipes)); bool result = true; - scheduler.Spawn([&](Coroutine& co) -> Task { + sched.Spawn([&](Coroutine& co) -> Task { result = co_await CheckPipeReady(co, pipes[0], 50000000ULL); close(pipes[0]); @@ -906,18 +906,18 @@ TEST(Cpp20WaitTimeout, ValueTaskWithTimeout) { co_return; }, "test"); - scheduler.Run(); + sched.Run(); EXPECT_FALSE(result); } TEST(Cpp20WaitTimeout, DataArrivesDuringWait) { - Scheduler scheduler; + Scheduler sched; int pipes[2]; ASSERT_EQ(0, pipe(pipes)); bool success = false; - scheduler.Spawn([&](Coroutine& co) -> Task { + sched.Spawn([&](Coroutine& co) -> Task { // Spawn a helper that writes after a short delay. co_await co.Yield(); @@ -929,14 +929,14 @@ TEST(Cpp20WaitTimeout, DataArrivesDuringWait) { co_return; }, "waiter"); - scheduler.Spawn([&](Coroutine& co) -> Task { + sched.Spawn([&](Coroutine& co) -> Task { co_await co.Sleep(std::chrono::milliseconds(20)); char c = 'x'; (void)write(pipes[1], &c, 1); co_return; }, "writer"); - scheduler.Run(); + sched.Run(); EXPECT_TRUE(success); } diff --git a/http_client/main.cc b/http_client/main.cc index 9820be8..ed1e36c 100644 --- a/http_client/main.cc +++ b/http_client/main.cc @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -28,7 +29,7 @@ void Usage(void) { // Send data to the server from a coroutine. static bool SendToServer(co::Coroutine *c, int fd, const char *request, size_t length) { - int offset = 0; + size_t offset = 0; const size_t kMaxLength = 1024; while (length > 0) { // Wait until we can send to the network. This will yield to other @@ -46,8 +47,8 @@ static bool SendToServer(co::Coroutine *c, int fd, const char *request, if (n == 0) { return false; } - length -= n; - offset += n; + length -= static_cast(n); + offset += static_cast(n); } return true; } @@ -94,7 +95,8 @@ static size_t ReadHeaders(std::string &buffer, std::vector &header, char *name = &buffer[i]; while (i < buffer.size() && buffer[i] != ':') { // Convert name to upper case as they are case insensitive. - buffer[i] = toupper(buffer[i]); + buffer[i] = static_cast( + ::toupper(static_cast(buffer[i]))); i++; } // No header value, end of headers. @@ -109,7 +111,7 @@ static size_t ReadHeaders(std::string &buffer, std::vector &header, } char *value = &buffer[i]; while (i < buffer.size()) { - if (i < buffer.size() + 3 && buffer[i] == '\r') { + if (i + 2 < buffer.size() && buffer[i] == '\r') { // Check for continuation with a space as the first character on the // next line. TAB too. if (buffer[i + 2] != ' ' && buffer[i + 2] != '\t') { @@ -140,7 +142,7 @@ static size_t ReadContents(co::Coroutine *c, int fd, std::string &buffer, if (write_to_output) { fwrite(&buffer[i], 1, nbytes, stdout); } - length -= nbytes; + length -= static_cast(nbytes); i += nbytes; } else { // No data in buffer, read some more into the buffer. @@ -157,7 +159,7 @@ static size_t ReadContents(co::Coroutine *c, int fd, std::string &buffer, printf("done\n"); break; } - buffer += std::string(buf, n); + buffer += std::string(buf, static_cast(n)); } } return i; @@ -168,7 +170,8 @@ static size_t ReadChunkLength(co::Coroutine *c, int fd, std::string &buffer, for (;;) { char ch; if (i < buffer.size()) { - ch = toupper(buffer[i++]); + ch = static_cast( + ::toupper(static_cast(buffer[i++]))); } else { // Fill the buffer with some more data. buffer.clear(); @@ -185,19 +188,20 @@ static size_t ReadChunkLength(co::Coroutine *c, int fd, std::string &buffer, // Didn't read anything, EOF on input. return i; } - buffer += std::string(buf, n); + buffer += std::string(buf, static_cast(n)); continue; } if (ch == '\r') { i++; break; } + int digit; if (ch > '9') { - ch = ch - 'A' + 10; + digit = ch - 'A' + 10; } else { - ch -= '0'; + digit = ch - '0'; } - *length = (*length << 4) | ch; + *length = (*length << 4) | digit; } return i; } @@ -233,7 +237,7 @@ void Client(co::Coroutine *c, std::string server_name, in_addr_t ipaddr, .sin_len = sizeof(int), #endif .sin_addr = {.s_addr = ipaddr}}; - int e = connect(fd, (struct sockaddr *)&addr, sizeof(addr)); + int e = connect(fd, reinterpret_cast(&addr), sizeof(addr)); if (e != 0) { close(fd); perror("connect"); @@ -244,7 +248,12 @@ void Client(co::Coroutine *c, std::string server_name, in_addr_t ipaddr, int reqlen = snprintf(request, sizeof(request), "GET %s HTTP/1.1\r\nHost: %s\r\n\r\n", filename.c_str(), server_name.c_str()); - bool ok = SendToServer(c, fd, request, reqlen); + if (reqlen < 0 || static_cast(reqlen) >= sizeof(request)) { + fprintf(stderr, "Failed to format HTTP request\n"); + close(fd); + return; + } + bool ok = SendToServer(c, fd, request, static_cast(reqlen)); if (!ok) { fprintf(stderr, "Failed to send to server: %s\n", strerror(errno)); close(fd); @@ -272,7 +281,7 @@ void Client(co::Coroutine *c, std::string server_name, in_addr_t ipaddr, return; } // Append to data buffer. - buffer += std::string(buf, n); + buffer += std::string(buf, static_cast(n)); // A blank line terminates the read. if (buffer.find("\r\n\r\n") != std::string::npos) { @@ -300,8 +309,8 @@ void Client(co::Coroutine *c, std::string server_name, in_addr_t ipaddr, fprintf(stderr, "%s Error: %d: ", protocol.c_str(), status_value); // Print all error strings. const char *sep = ""; - for (size_t i = kError; i < header.size(); i++) { - std::string &s = header[i]; + for (size_t err_idx = kError; err_idx < header.size(); err_idx++) { + std::string &s = header[err_idx]; fprintf(stderr, "%s%s", sep, s.c_str()); sep = " "; } @@ -319,9 +328,16 @@ void Client(co::Coroutine *c, std::string server_name, in_addr_t ipaddr, if (it != http_headers.end() && it->second == "chunked") { is_chunked = true; } else { - auto it = http_headers.find("CONTENT-LENGTH"); - if (it != http_headers.end()) { - content_length = (int)strtoll(it->second.c_str(), NULL, 10); + auto content_length_it = http_headers.find("CONTENT-LENGTH"); + if (content_length_it != http_headers.end()) { + char *end = nullptr; + const long long parsed_length = strtoll( + content_length_it->second.c_str(), &end, 10); + if (end != content_length_it->second.c_str() && *end == '\0' && + parsed_length >= 0 && + parsed_length <= std::numeric_limits::max()) { + content_length = static_cast(parsed_length); + } } } @@ -385,7 +401,13 @@ int main(int argc, const char *argv[]) { fprintf(stderr, "unknown host %s\n", host.c_str()); exit(1); } - in_addr_t ipaddr = ((struct in_addr *)entry->h_addr_list[0])->s_addr; + in_addr host_addr{}; + if (entry->h_length != static_cast(sizeof(host_addr))) { + fprintf(stderr, "unexpected address length for host %s\n", host.c_str()); + exit(1); + } + std::memcpy(&host_addr, entry->h_addr_list[0], sizeof(host_addr)); + in_addr_t ipaddr = host_addr.s_addr; co::CoroutineScheduler scheduler; std::set> jobs; diff --git a/http_server/main.cc b/http_server/main.cc index 445b0b4..f19395c 100644 --- a/http_server/main.cc +++ b/http_server/main.cc @@ -4,6 +4,7 @@ #include "co/coroutine.h" #include +#include #include #include #include @@ -32,7 +33,7 @@ void Signal(int sig) { // Send a buffer full of data to the coroutines file descriptor. static void SendToClient(co::Coroutine *c, int fd, const char *response, size_t length) { - int offset = 0; + size_t offset = 0; const size_t kMaxLength = 1024; while (length > 0) { // Wait until we can send to the network. This will yield to other @@ -50,8 +51,8 @@ static void SendToClient(co::Coroutine *c, int fd, const char *response, if (n == 0) { return; } - length -= n; - offset += n; + length -= static_cast(n); + offset += static_cast(n); } } @@ -97,7 +98,8 @@ static void ReadHeaders(std::string &buffer, std::vector &header, char *name = &buffer[i]; while (i < buffer.size() && buffer[i] != ':') { // Convert name to upper case as they are case insensitive. - buffer[i] = toupper(buffer[i]); + buffer[i] = static_cast( + ::toupper(static_cast(buffer[i]))); i++; } // No header value, end of headers. @@ -112,7 +114,7 @@ static void ReadHeaders(std::string &buffer, std::vector &header, } char *value = &buffer[i]; while (i < buffer.size()) { - if (i < buffer.size() + 3 && buffer[i] == '\r') { + if (i + 2 < buffer.size() && buffer[i] == '\r') { // Check for continuation with a space as the first character on the // next line. TAB too. if (buffer[i + 2] != ' ' && buffer[i + 2] != '\t') { @@ -130,8 +132,8 @@ static void ReadHeaders(std::string &buffer, std::vector &header, } } -void Server(co::Coroutine *c, int fd, struct sockaddr_in sender, - socklen_t sender_len) { +void Server(co::Coroutine *c, int fd, [[maybe_unused]] struct sockaddr_in sender, + [[maybe_unused]] socklen_t sender_len) { std::string buffer; // Read incoming HTTP request and parse it. @@ -155,7 +157,7 @@ void Server(co::Coroutine *c, int fd, struct sockaddr_in sender, return; } // Append to data buffer. - buffer += std::string(buf, n); + buffer += std::string(buf, static_cast(n)); // A blank line terminates the read if (buffer.find("\r\n\r\n") != std::string::npos) { @@ -198,34 +200,40 @@ void Server(co::Coroutine *c, int fd, struct sockaddr_in sender, if (e == -1) { int n = snprintf(response, sizeof(response), "%s 404 Not Found\r\n\r\n", protocol.c_str()); - SendToClient(c, fd, response, n); + if (n > 0 && static_cast(n) < sizeof(response)) { + SendToClient(c, fd, response, static_cast(n)); + } } else { int file_fd = open(filename.c_str(), O_RDONLY); if (file_fd == -1) { int n = snprintf(response, sizeof(response), "%s 404 Not Found\r\n\r\n", protocol.c_str()); - SendToClient(c, fd, response, n); + if (n > 0 && static_cast(n) < sizeof(response)) { + SendToClient(c, fd, response, static_cast(n)); + } } else { // Send the file back. - int n = - snprintf(response, sizeof(response), - "%s 200 OK\r\nContent-type: text/html\r\nContent-length: " - "%zd\r\n\r\n", - protocol.c_str(), static_cast(st.st_size)); - SendToClient(c, fd, response, n); + int n = snprintf( + response, sizeof(response), + "%s 200 OK\r\nContent-type: text/html\r\nContent-length: " + "%jd\r\n\r\n", + protocol.c_str(), static_cast(st.st_size)); + if (n > 0 && static_cast(n) < sizeof(response)) { + SendToClient(c, fd, response, static_cast(n)); + } for (;;) { char buf[1024]; c->Wait(file_fd, POLLIN); - ssize_t n = read(file_fd, buf, sizeof(buf)); - if (n == -1) { + ssize_t bytes_read = read(file_fd, buf, sizeof(buf)); + if (bytes_read == -1) { perror("file read"); break; } - if (n == 0) { + if (bytes_read == 0) { break; } - SendToClient(c, fd, buf, n); + SendToClient(c, fd, buf, static_cast(bytes_read)); } close(file_fd); } @@ -234,7 +242,9 @@ void Server(co::Coroutine *c, int fd, struct sockaddr_in sender, // Invalid request method. int n = snprintf(response, sizeof(response), "%s 400 Invalid request method\r\n\r\n", protocol.c_str()); - SendToClient(c, fd, response, n); + if (n > 0 && static_cast(n) < sizeof(response)) { + SendToClient(c, fd, response, static_cast(n)); + } } close(fd); @@ -254,7 +264,7 @@ void Listener(co::Coroutine *c) { .sin_len = sizeof(int), #endif .sin_addr = {.s_addr = INADDR_ANY}}; - int e = bind(s, (struct sockaddr *)&addr, sizeof(addr)); + int e = bind(s, reinterpret_cast(&addr), sizeof(addr)); if (e == -1) { perror("bind"); close(s); @@ -283,7 +293,7 @@ void Listener(co::Coroutine *c) { struct sockaddr_in sender; socklen_t sender_len = sizeof(sender); - int fd = accept(s, (struct sockaddr *)&sender, &sender_len); + int fd = accept(s, reinterpret_cast(&sender), &sender_len); if (fd == -1) { perror("accept"); continue; @@ -297,7 +307,7 @@ void Listener(co::Coroutine *c) { } } -int main(int argc, const char *argv[]) { +int main(int /*argc*/, const char * /*argv*/[]) { co::CoroutineScheduler scheduler; g_scheduler = &scheduler; // For signal handler.