Skip to content

Improve encapsulation of TextBasedProtocol fields.#2187

Open
Dimi1010 wants to merge 2 commits into
seladb:devfrom
Dimi1010:refactor/text-protocol-encapsulation
Open

Improve encapsulation of TextBasedProtocol fields.#2187
Dimi1010 wants to merge 2 commits into
seladb:devfrom
Dimi1010:refactor/text-protocol-encapsulation

Conversation

@Dimi1010

Copy link
Copy Markdown
Collaborator

The PR changes the visibility of the fields of TextBasedProtocol from protected to private and establishes methods to allow checked mutation. Notably all offsets are range validated prior to change.

…hrough methods instead of direct value mutation.

# Conflicts:
#	Packet++/header/TextBasedProtocol.h
@Dimi1010
Dimi1010 requested a review from seladb as a code owner July 14, 2026 15:54
Comment on lines +388 to +394
void TextBasedProtocolMessage::validateFieldsOffset(int offset) const
{
if (offset < 0 || offset > m_DataLen)
throw std::invalid_argument(
"Field offset must be greater or equal to zero and less than or equal to the data length");
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should be std::out_of_range?

@codecov

codecov Bot commented Jul 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.23529% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.66%. Comparing base (f0df1d1) to head (77d337c).
⚠️ Report is 1 commits behind head on dev.

Files with missing lines Patch % Lines
Packet++/src/TextBasedProtocol.cpp 89.47% 2 Missing ⚠️
Packet++/src/HttpLayer.cpp 80.00% 0 Missing and 1 partial ⚠️
Packet++/src/SdpLayer.cpp 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##              dev    #2187   +/-   ##
=======================================
  Coverage   82.66%   82.66%           
=======================================
  Files         332      332           
  Lines       60154    60161    +7     
  Branches    12571    12558   -13     
=======================================
+ Hits        49724    49731    +7     
+ Misses       9021     9017    -4     
- Partials     1409     1413    +4     
Flag Coverage Δ
23.11.6 7.26% <22.22%> (-0.05%) ⬇️
24.11.5 7.30% <22.22%> (+<0.01%) ⬆️
25.11.1 7.26% <22.22%> (-0.05%) ⬇️
alpine320 76.87% <86.66%> (-0.01%) ⬇️
fedora42 76.42% <85.18%> (-0.03%) ⬇️
macos-15 82.26% <85.18%> (-0.01%) ⬇️
macos-26 82.28% <85.18%> (-0.01%) ⬇️
macos-26-intel 82.19% <85.18%> (-0.01%) ⬇️
mingw32 70.97% <85.18%> (+<0.01%) ⬆️
mingw64 70.94% <85.18%> (+0.06%) ⬆️
npcap ?
rhel94 76.23% <82.14%> (-0.04%) ⬇️
ubuntu2204 76.27% <82.14%> (-0.04%) ⬇️
ubuntu2404 76.58% <85.18%> (-0.01%) ⬇️
ubuntu2604 76.51% <85.18%> (+<0.01%) ⬆️
ubuntu2604-arm64 76.35% <86.66%> (-0.03%) ⬇️
ubuntu2604-icpx 59.05% <44.44%> (-0.01%) ⬇️
unittest 82.66% <88.23%> (+<0.01%) ⬆️
windows-2022 85.81% <90.00%> (+0.11%) ⬆️
windows-2025 85.55% <90.00%> (+0.12%) ⬆️
winpcap 85.84% <90.00%> (+0.20%) ⬆️
xdp 52.90% <82.14%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant