feat: S24.09 StreamSender bad-setup contract — reject NULL Resolver/Stream/Address#455
Conversation
Introduces StreamSender_IsValidConfig in the UdpSender shape and routes _Create through it. NULL config now emits SolidSyslog_Error(ERROR, STREAMSENDER_ERROR_NULL_CONFIG) and resolves to the shared SolidSyslogNullSender — no pool slot consumed. This is the first of four NULL-path slices that bring StreamSender to the same bad-setup contract as UdpSender (S12.06).
…Sender_Create Extends StreamSender_IsValidConfig with the three remaining NULL paths under the bad-setup contract: NULL Resolver, NULL Stream, NULL Address each emit SolidSyslog_Error(ERROR) with the matching STREAMSENDER_ERROR_NULL_* code and resolve to the shared SolidSyslogNullSender. Order matches SolidSyslogStreamSenderConfig field order.
…andle Mirrors the UdpSenderBadSetup pair: a sender returned from SolidSyslogStreamSender_Create on a bad config is the shared SolidSyslogNullSender, so Send returns true (drop-on-floor) and Disconnect is a safe no-op.
CLAUDE.md's StreamSender row now mirrors the UdpSender row's wording — both pool exhaustion and bad config (NULL config / Resolver / Stream / Address) resolve to the shared SolidSyslogNullSender.
|
Warning Review limit reached
Your plan includes 1 review of capacity. Refill in 18 minutes and 46 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR implements the bad-setup validation contract for ChangesStreamSender bad-setup validation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1344 passed) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Tests/SolidSyslogStreamSenderTest.cpp (1)
771-808: ⚡ Quick winDRY the repeated bad-setup assertions with a
CHECK_*macro/helper.The four NULL-path tests duplicate the same severity/source/code assertion shape; extracting one
CHECK_STREAMSENDER_BAD_SETUP_ERROR(expectedCode)macro would align with the test-style rules and reduce drift risk.As per coding guidelines: “Use
CHECK_*macros for repeated assertion shapes in test code” and “DRY the setup, DRY the asserts, keep the test body small.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/SolidSyslogStreamSenderTest.cpp` around lines 771 - 808, The four tests (SolidSyslogStreamSenderBadSetup::CreateWithNullConfigReportsError, ::CreateWithNullResolverReportsError, ::CreateWithNullStreamReportsError, ::CreateWithNullAddressReportsError) duplicate the same post-call assertions; create a CHECK_STREAMSENDER_BAD_SETUP_ERROR(expectedCode) test helper macro that encapsulates the three assertions (CALLED_FAKE(ErrorHandlerFake_Handle, ONCE), LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_ERROR, ErrorHandlerFake_LastSeverity()), POINTERS_EQUAL(&StreamSenderErrorSource, ErrorHandlerFake_LastSource()), and UNSIGNED_LONGS_EQUAL(expectedCode, ErrorHandlerFake_LastCode())) and replace the repeated assertion blocks in each test with a single call to that macro immediately after the SolidSyslogStreamSender_Create(...) invocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Tests/SolidSyslogStreamSenderTest.cpp`:
- Around line 771-808: The four tests
(SolidSyslogStreamSenderBadSetup::CreateWithNullConfigReportsError,
::CreateWithNullResolverReportsError, ::CreateWithNullStreamReportsError,
::CreateWithNullAddressReportsError) duplicate the same post-call assertions;
create a CHECK_STREAMSENDER_BAD_SETUP_ERROR(expectedCode) test helper macro that
encapsulates the three assertions (CALLED_FAKE(ErrorHandlerFake_Handle, ONCE),
LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_ERROR, ErrorHandlerFake_LastSeverity()),
POINTERS_EQUAL(&StreamSenderErrorSource, ErrorHandlerFake_LastSource()), and
UNSIGNED_LONGS_EQUAL(expectedCode, ErrorHandlerFake_LastCode())) and replace the
repeated assertion blocks in each test with a single call to that macro
immediately after the SolidSyslogStreamSender_Create(...) invocation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 229081f4-d90b-414f-bc76-fd0ad1b6c833
📒 Files selected for processing (5)
CLAUDE.mdCore/Interface/SolidSyslogStreamSenderErrors.hCore/Source/SolidSyslogStreamSenderMessages.cCore/Source/SolidSyslogStreamSenderStatic.cTests/SolidSyslogStreamSenderTest.cpp
- IWYU: drop unused #include "SolidSyslogError.h". SOLIDSYSLOG_SEVERITY_ERROR comes from SolidSyslogPrival.h; &StreamSenderErrorSource only needs the struct forward-decl from SolidSyslogStreamSenderErrors.h. - cppcheck: suppress unreadVariable on the BadSetup setup()'s config assignment, matching the existing pattern in the SolidSyslogStreamSenderDestroy group — cppcheck does not model CppUTest macros so the field appears unread inside setup().
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1344 passed) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
Each of the four NULL-path tests in SolidSyslogStreamSenderBadSetup repeated the same four-line assertion block (CALLED_FAKE handle count, severity, source pointer, error code). Lift them into a single CHECK_STREAMSENDER_BAD_SETUP_ERROR(expectedCode) macro — same shape as CHECK_PRIVAL / CHECK_BLOCK_CONTAINS elsewhere in the test suite. Macro (not function) so failures report the caller's __FILE__/__LINE__; do/while(0) wrapped for safe single-statement use.
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1344 passed) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
1 participant
Summary
Brings
SolidSyslogStreamSender_Createto the same bad-setup posture asSolidSyslogUdpSender_Create(S12.06). NULLconfig,Resolver,Stream, orAddressnow each emit a distinctSolidSyslog_Error(SEVERITY_ERROR, …)and resolve to the sharedSolidSyslogNullSenderwithout consuming a pool slot.STREAMSENDER_ERROR_NULL_{CONFIG,RESOLVER,STREAM,ADDRESS}— naming matches theUDPSENDER_ERROR_NULL_*convention, ordered to match theSolidSyslogStreamSenderConfigfield order._Createnow routes through a newStreamSender_IsValidConfighelper modelled onUdpSender_IsValidConfig(SolidSyslogUdpSenderStatic.c:48-76).SolidSyslogStreamSenderBadSetupTEST_GROUP mirrors theSolidSyslogUdpSenderBadSetuppair — one test per NULL path, plus crash-safety tests provingSend(drops on floor, returnstrue) andDisconnect(no-op) are safe through the returned NullSender handle.SolidSyslogStreamSender.hrow tail now reads "Pool-exhaustion and bad-config fallback is the sharedSolidSyslogNullSender" — matching the UdpSender row.SolidSyslogStreamSenderConfig.Endpointis intentionally left optional (existingNoEndpointConfiguredConnectsToPortZerotest pins this), so unlike UdpSender there is noNULL_ENDPOINTcode.Slice trail
Test plan
build-linux-gcc— full StreamSender suite green (StreamSender, Destroy, Config, Failure, Pool, BadSetup)build-linux-clangsanitize-linux-gcc— ASan/UBSan clean across all StreamSender groupsanalyze-formatanalyze-tidyanalyze-cppcheck(standard)cppcheck-misra(CI invocation verbatim) — clean, nomisra_suppressions.txtchangesbuild-windows-msvc,integration-{linux,windows}-openssl, BDD jobs,analyze-iwyuCloses #424
Summary by CodeRabbit
Documentation
Bug Fixes
Tests