diff options
| author | Stefan Boberg <[email protected]> | 2026-03-06 10:11:51 +0100 |
|---|---|---|
| committer | GitHub Enterprise <[email protected]> | 2026-03-06 10:11:51 +0100 |
| commit | 1e731796187ad73b2dee44b48fcecdd487616394 (patch) | |
| tree | 0ea37769f743ae1fb2eacc37bc8ccfa88ecc0d64 /src/zencore/include | |
| parent | fix oidctoken exe lookup check (#811) (diff) | |
| download | zen-1e731796187ad73b2dee44b48fcecdd487616394.tar.xz zen-1e731796187ad73b2dee44b48fcecdd487616394.zip | |
Claude config, some bug fixes (#813)
* Claude config updates
* Bug fixes and hardening across `zencore` and `zenhttp`, identified via static analysis.
### zencore
- **`ZEN_ASSERT` macro** -- extended to accept an optional string message literal; added `ZEN_ASSERT_MSG_` helper for message formatting. Callers needing runtime fmt-style formatting should use `ZEN_ASSERT_FORMAT`.
- **`MpscQueue`** -- fixed `TypeCompatibleStorage` to use a properly-sized `char Storage[sizeof(T)]` array instead of a single `char`; corrected `Data()` to cast `&Storage` rather than `this`; switched cache-line alignment to a fixed constant to avoid GCC's `-Winterference-size` warning. Enabled previously-disabled tests.
- **`StringBuilderImpl`** -- initialized `m_Base`/`m_CurPos`/`m_End` to `nullptr`. Fixed `StringCompare` return type (`bool` -> `int`). Fixed `ParseInt` to reject strings with trailing non-numeric characters. Removed deprecated `<codecvt>` include.
- **`NiceNumGeneral`** -- replaced `powl()` with integer `IntPow()` to avoid floating-point precision issues.
- **`RwLock::ExclusiveLockScope`** -- added move constructor/assignment; initialized `m_Lock` to `nullptr`.
- **`Latch::AddCount`** -- fixed variable type (`std::atomic_ptrdiff_t` -> `std::ptrdiff_t` for the return value of `fetch_add`).
- **`thread.cpp`** -- fixed Linux `pthread_setname_np` 16-byte name truncation; added null check before dereferencing in `Event::Close()`; fixed `NamedEvent::Close()` to call `close(Fd)` outside the lock region; added null guard in `NamedMutex` destructor; `Sleep()` now returns early for non-positive durations.
- **`MD5Stream`** -- was entirely stubbed out (no-op); now correctly calls `MD5Init`/`MD5Update`/`MD5Final`. Fixed `ToHexString` to use the correct string length. Fixed forward declarations. Fixed tests to compare `compare() == 0`.
- **`sentryintegration.cpp`** -- guard against null `filename`/`funcname` in spdlog message handler to prevent a crash in `fmt::format`.
- **`jobqueue.cpp`** -- fixed lost job ID when `IdGenerator` wraps around zero; fixed raw `Job*` in `RunningJobs` map (potential use-after-free) to `RefPtr<Job>`; fixed range-loop copies; fixed format string typo.
- **`trace.cpp`** -- suppress GCC false-positive warnings in third-party `trace.h` include.
### zenhttp
- **WebSocket close race** (`wsasio`, `wshttpsys`, `httpwsclient`) -- `m_CloseSent` promoted from `bool` to `std::atomic<bool>`; close check changed to `exchange(true)` to eliminate the check-then-set data race.
- **`wsframecodec.cpp`** -- reject WebSocket frames with payload > 256 MB to prevent OOM from malformed/malicious frames.
- **`oidc.cpp`** -- URL-encode refresh token and client ID in token requests (`FormUrlEncode`); parse `end_session_endpoint` and `device_authorization_endpoint` from OIDC discovery document.
- **`httpclientcommon.cpp`** -- propagate error code from `AppendData` when flushing the cache buffer.
- **`httpclient.h`** -- initialize all uninitialized members (`ErrorCode`, `UploadedBytes`, `DownloadedBytes`, `ElapsedSeconds`, `MultipartBoundary` fields).
- **`httpserver.h`** -- fix `operator=` return type for `HttpRpcHandler` (missing `&`).
- **`packageformat.h`** -- fix `~0u` (32-bit truncation) to `~uint64_t(0)` for a `uint64_t` field.
- **`httpparser`** -- initialize `m_RequestVerb` in both declaration and `ResetState()`.
- **`httpplugin.cpp`** -- initialize `m_BasePort`; fix format string missing quotes around connection name.
- **`httptracer.h`** -- move `#pragma once` before includes.
- **`websocket.h`** -- initialize `WebSocketMessage::Opcode`.
### zenserver
- **`hubservice.cpp`** -- fix two `ZEN_ASSERT` calls that incorrectly used fmt-style format args; converted to `ZEN_ASSERT_FORMAT`.
Diffstat (limited to 'src/zencore/include')
| -rw-r--r-- | src/zencore/include/zencore/md5.h | 2 | ||||
| -rw-r--r-- | src/zencore/include/zencore/mpscqueue.h | 20 | ||||
| -rw-r--r-- | src/zencore/include/zencore/string.h | 22 | ||||
| -rw-r--r-- | src/zencore/include/zencore/thread.h | 16 | ||||
| -rw-r--r-- | src/zencore/include/zencore/xxhash.h | 2 | ||||
| -rw-r--r-- | src/zencore/include/zencore/zencore.h | 34 |
6 files changed, 60 insertions, 36 deletions
diff --git a/src/zencore/include/zencore/md5.h b/src/zencore/include/zencore/md5.h index d934dd86b..3b0b7cae6 100644 --- a/src/zencore/include/zencore/md5.h +++ b/src/zencore/include/zencore/md5.h @@ -43,6 +43,8 @@ public: MD5 GetHash(); private: + // Opaque storage for MD5_CTX (104 bytes, aligned to uint32_t) + alignas(4) uint8_t m_Context[104]; }; void md5_forcelink(); // internal diff --git a/src/zencore/include/zencore/mpscqueue.h b/src/zencore/include/zencore/mpscqueue.h index 19e410d85..d97c433fd 100644 --- a/src/zencore/include/zencore/mpscqueue.h +++ b/src/zencore/include/zencore/mpscqueue.h @@ -22,10 +22,10 @@ namespace zen { template<typename ElementType> struct TypeCompatibleStorage { - ElementType* Data() { return (ElementType*)this; } - const ElementType* Data() const { return (const ElementType*)this; } + ElementType* Data() { return reinterpret_cast<ElementType*>(&Storage); } + const ElementType* Data() const { return reinterpret_cast<const ElementType*>(&Storage); } - alignas(ElementType) char DataMember; + alignas(ElementType) char Storage[sizeof(ElementType)]; }; /** Fast multi-producer/single-consumer unbounded concurrent queue. @@ -58,7 +58,7 @@ public: Tail = Next; Next = Tail->Next.load(std::memory_order_relaxed); - std::destroy_at((ElementType*)&Tail->Value); + std::destroy_at(Tail->Value.Data()); delete Tail; } } @@ -67,7 +67,7 @@ public: void Enqueue(ArgTypes&&... Args) { Node* New = new Node; - new (&New->Value) ElementType(std::forward<ArgTypes>(Args)...); + new (New->Value.Data()) ElementType(std::forward<ArgTypes>(Args)...); Node* Prev = Head.exchange(New, std::memory_order_acq_rel); Prev->Next.store(New, std::memory_order_release); @@ -82,7 +82,7 @@ public: return {}; } - ElementType* ValuePtr = (ElementType*)&Next->Value; + ElementType* ValuePtr = Next->Value.Data(); std::optional<ElementType> Res{std::move(*ValuePtr)}; std::destroy_at(ValuePtr); @@ -100,9 +100,11 @@ private: }; private: - std::atomic<Node*> Head; // accessed only by producers - alignas(hardware_constructive_interference_size) - Node* Tail; // accessed only by consumer, hence should be on a different cache line than `Head` + // Use a fixed constant to avoid GCC's -Winterference-size warning with std::hardware_destructive_interference_size + static constexpr std::size_t CacheLineSize = 64; + + alignas(CacheLineSize) std::atomic<Node*> Head; // accessed only by producers + alignas(CacheLineSize) Node* Tail; // accessed only by consumer, separate cache line from Head }; void mpscqueue_forcelink(); diff --git a/src/zencore/include/zencore/string.h b/src/zencore/include/zencore/string.h index 250eb9f56..4deca63ed 100644 --- a/src/zencore/include/zencore/string.h +++ b/src/zencore/include/zencore/string.h @@ -8,7 +8,6 @@ #include <stdint.h> #include <string.h> #include <charconv> -#include <codecvt> #include <compare> #include <concepts> #include <optional> @@ -51,7 +50,7 @@ StringLength(const wchar_t* str) return wcslen(str); } -inline bool +inline int StringCompare(const char16_t* s1, const char16_t* s2) { char16_t c1, c2; @@ -66,7 +65,7 @@ StringCompare(const char16_t* s1, const char16_t* s2) ++s1; ++s2; } - return uint16_t(c1) - uint16_t(c2); + return int(uint16_t(c1)) - int(uint16_t(c2)); } inline bool @@ -122,10 +121,10 @@ public: StringBuilderImpl() = default; ~StringBuilderImpl(); - StringBuilderImpl(const StringBuilderImpl&) = delete; - StringBuilderImpl(const StringBuilderImpl&&) = delete; + StringBuilderImpl(const StringBuilderImpl&) = delete; + StringBuilderImpl(StringBuilderImpl&&) = delete; const StringBuilderImpl& operator=(const StringBuilderImpl&) = delete; - const StringBuilderImpl& operator=(const StringBuilderImpl&&) = delete; + StringBuilderImpl& operator=(StringBuilderImpl&&) = delete; inline size_t AddUninitialized(size_t Count) { @@ -374,9 +373,9 @@ protected: [[noreturn]] void Fail(const char* FailReason); // note: throws exception - C* m_Base; - C* m_CurPos; - C* m_End; + C* m_Base = nullptr; + C* m_CurPos = nullptr; + C* m_End = nullptr; bool m_IsDynamic = false; bool m_IsExtendable = false; }; @@ -773,8 +772,9 @@ std::optional<T> ParseInt(const std::string_view& Input) { T Out = 0; - const std::from_chars_result Result = std::from_chars(Input.data(), Input.data() + Input.size(), Out); - if (Result.ec == std::errc::invalid_argument || Result.ec == std::errc::result_out_of_range) + const char* End = Input.data() + Input.size(); + const std::from_chars_result Result = std::from_chars(Input.data(), End, Out); + if (Result.ec == std::errc::invalid_argument || Result.ec == std::errc::result_out_of_range || Result.ptr != End) { return std::nullopt; } diff --git a/src/zencore/include/zencore/thread.h b/src/zencore/include/zencore/thread.h index a1c68b0b2..d0d710ee8 100644 --- a/src/zencore/include/zencore/thread.h +++ b/src/zencore/include/zencore/thread.h @@ -58,7 +58,7 @@ public: } private: - RwLock* m_Lock; + RwLock* m_Lock = nullptr; }; inline auto WithSharedLock(auto&& Fun) @@ -69,6 +69,16 @@ public: struct ExclusiveLockScope { + ExclusiveLockScope(const ExclusiveLockScope& Rhs) = delete; + ExclusiveLockScope(ExclusiveLockScope&& Rhs) : m_Lock(Rhs.m_Lock) { Rhs.m_Lock = nullptr; } + ExclusiveLockScope& operator=(ExclusiveLockScope&& Rhs) + { + ReleaseNow(); + m_Lock = Rhs.m_Lock; + Rhs.m_Lock = nullptr; + return *this; + } + ExclusiveLockScope& operator=(const ExclusiveLockScope& Rhs) = delete; ExclusiveLockScope(RwLock& Lock) : m_Lock(&Lock) { Lock.AcquireExclusive(); } ~ExclusiveLockScope() { ReleaseNow(); } @@ -82,7 +92,7 @@ public: } private: - RwLock* m_Lock; + RwLock* m_Lock = nullptr; }; inline auto WithExclusiveLock(auto&& Fun) @@ -195,7 +205,7 @@ public: // false positive completion results. void AddCount(std::ptrdiff_t Count) { - std::atomic_ptrdiff_t Old = Counter.fetch_add(Count); + std::ptrdiff_t Old = Counter.fetch_add(Count); ZEN_ASSERT(Old > 0); } diff --git a/src/zencore/include/zencore/xxhash.h b/src/zencore/include/zencore/xxhash.h index fc55b513b..f79d39b61 100644 --- a/src/zencore/include/zencore/xxhash.h +++ b/src/zencore/include/zencore/xxhash.h @@ -87,7 +87,7 @@ struct XXH3_128Stream } private: - XXH3_state_s m_State; + XXH3_state_s m_State{}; }; struct XXH3_128Stream_deprecated diff --git a/src/zencore/include/zencore/zencore.h b/src/zencore/include/zencore/zencore.h index 177a19fff..a31950b0b 100644 --- a/src/zencore/include/zencore/zencore.h +++ b/src/zencore/include/zencore/zencore.h @@ -70,26 +70,36 @@ protected: } // namespace zen -#define ZEN_ASSERT(x, ...) \ - do \ - { \ - if (x) [[unlikely]] \ - break; \ - zen::AssertImpl::ExecAssert(__FILE__, __LINE__, __FUNCTION__, #x); \ +#define ZEN_ASSERT(x, ...) \ + do \ + { \ + if (x) [[unlikely]] \ + break; \ + zen::AssertImpl::ExecAssert(__FILE__, __LINE__, __FUNCTION__, ZEN_ASSERT_MSG_(#x, ##__VA_ARGS__)); \ } while (false) #ifndef NDEBUG -# define ZEN_ASSERT_SLOW(x, ...) \ - do \ - { \ - if (x) [[unlikely]] \ - break; \ - zen::AssertImpl::ExecAssert(__FILE__, __LINE__, __FUNCTION__, #x); \ +# define ZEN_ASSERT_SLOW(x, ...) \ + do \ + { \ + if (x) [[unlikely]] \ + break; \ + zen::AssertImpl::ExecAssert(__FILE__, __LINE__, __FUNCTION__, ZEN_ASSERT_MSG_(#x, ##__VA_ARGS__)); \ } while (false) #else # define ZEN_ASSERT_SLOW(x, ...) #endif +// Internal: select between "expr" and "expr: message" forms. +// With no extra args: ZEN_ASSERT_MSG_("expr") -> "expr" +// With a message arg: ZEN_ASSERT_MSG_("expr", "msg") -> "expr" ": " "msg" +// With fmt-style args: ZEN_ASSERT_MSG_("expr", "msg", args...) -> "expr" ": " "msg" +// The extra fmt args are silently discarded here — use ZEN_ASSERT_FORMAT for those. +#define ZEN_ASSERT_MSG_SELECT_(_1, _2, N, ...) N +#define ZEN_ASSERT_MSG_1_(expr) expr +#define ZEN_ASSERT_MSG_2_(expr, msg, ...) expr ": " msg +#define ZEN_ASSERT_MSG_(expr, ...) ZEN_ASSERT_MSG_SELECT_(unused, ##__VA_ARGS__, ZEN_ASSERT_MSG_2_, ZEN_ASSERT_MSG_1_)(expr, ##__VA_ARGS__) + ////////////////////////////////////////////////////////////////////////// #define ZEN_NOT_IMPLEMENTED(...) ZEN_ASSERT(false, __VA_ARGS__) |