Merged
Conversation
Replace the previous bounds check in EnsureAvailable with an unsigned comparison against a local available variable and extract the throw into a separate ThrowNotEnoughData method. The new helper is marked NoInlining and includes XML documentation. This reduces duplicated throw logic in hot paths and makes the bounds-check semantics clearer and more JIT-friendly.
Correct a typo in the XML documentation of LiteNetLib/Utils/NetDataReader.cs by replacing an invalid <see cre="class"/> attribute with <see langword="class"/>. This fixes documentation warnings and ensures the class reference renders correctly.
Replace direct instantiation and obj.Deserialize(this) with calls to existing Get(out ...) overloads in both struct and class INetSerializable Get<T> methods. This removes duplicated deserialization code and centralizes behavior without changing functionality.
Introduce IPv4Size and IPv6Size constants and consolidate IPv4/IPv6 branching in GetIPEndPoint. Read the IP version flag, compute the address size, ensure buffer availability, construct the IPAddress from a ReadOnlySpan, and advance the position. Removes duplicated code and magic numbers while preserving existing behavior.
Call EnsureAvailable(1) before reading from the internal buffer in NetDataReader.GetByte to prevent out-of-range reads and provide consistent error handling. The method still increments the read position after returning the byte.
Use checked() when computing byteLength and call EnsureAvailable(byteLength) before taking a span. This prevents silent integer overflow on length * sizeof(T) and verifies there are enough bytes available for MemoryMarshal.Cast and subsequent array creation in GetUnmanagedArray<T>().
Introduce a GuidSize constant (16) and use it in GetGuid() instead of hard-coded 16s. This improves readability and avoids repeated magic numbers when ensuring availability, constructing the Guid from the data span, and advancing the read position.
Introduce IPv4Size (4), IPv6Size (16) and GuidSize (16) constants in NetDataWriter.cs and replace magic numbers with these constants in Put(Guid) and when determining addressSize for IPEndPoint. This improves readability and maintainability by avoiding literal size values.
Add EnsureAvailable checks to both GetStringArray overloads to verify there are enough bytes for the array string lengths before reading (uses checked(length * sizeof(ushort)) to catch overflows). Also rename the local array variable from `arr` to `result` and return it, cleaning up the methods and improving safety against buffer overruns.
Replace direct calculation (_dataSize - _position) with the AvailableBytes property in GetRemainingBytesSpan and GetRemainingBytesMemory. This deduplicates logic, improves clarity, and ensures both methods use the same availability check when returning the remaining data slice before advancing the position.
Avoid allocating a new byte[] when no data remains by returning Array.Empty<byte>() for size == 0. Also rename the temporary buffer to result for clarity; behavior for non-zero reads is unchanged (copy remaining bytes, advance position to end). This reduces allocations for empty reads.
Replace the previous platform-specific unsafe read logic in GetEnum<T>() with a direct call to GetUnmanaged<T>(). This centralizes unmanaged enum reading, removes the NET8 conditional (Unsafe.ReadUnaligned / fixed pointer) and reduces duplicated unsafe buffer-reading code without changing behavior.
Replace casting from GetByte() with a direct call to GetSByte() in LiteNetLib/Utils/NetDataReader.cs. This ensures signed byte values are read using the dedicated reader method (preserving correct sign/semantics) and keeps the reading logic consistent.
Move GetByte and GetSByte methods within LiteNetLib/Utils/NetDataReader.cs so they are grouped with other primitive read methods (placed before GetBool). This is a non-functional refactor to improve code organization and readability; no logic changes were made.
Refactor PeekSByte, PeekBool and PeekChar to call existing PeekByte()/PeekUShort() instead of directly accessing _data or PeekUnmanaged<char>(). This centralizes byte/ushort read logic (and any bounds/validation behavior) to reduce duplication and potential subtle bugs while preserving external behavior.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates LiteNetLib’s binary reader/writer utilities to replace magic numbers with named size constants, tighten bounds checks, and refactor several read/try-read paths for consistency and reduced duplication.
Changes:
- Introduced
IPv4Size/IPv6Size/GuidSizeconstants and applied them in reader/writer logic. - Refactored
NetDataReaderbounds checking (EnsureAvailable) to a fast-path with a non-inlined throw helper and addedTryGetUnmanaged<T>(). - Improved several read APIs (e.g.,
GetBytenow validates availability, rollback behavior inTryGetStringArray, overflow-checked byte counts for unmanaged arrays).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| LiteNetLib/Utils/NetDataWriter.cs | Replaces hard-coded sizes for GUID/IP serialization with named constants. |
| LiteNetLib/Utils/NetDataReader.cs | Refactors bounds checking and multiple getters/peekers/try-getters; adds TryGetUnmanaged<T>() and applies size constants. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.