[FIX] networking.c: replace DEBUG_OUT integer toggle with CMake opt-in; fix error messages using printf() instead of fprintf(stderr)#2175
Conversation
…o fprintf(stderr) - Remove #define DEBUG_OUT 0 from networking.c and replace all #if DEBUG_OUT guards with #ifdef NETWORKING_DEBUG, consistent with how other debug flags work in the codebase (see CCExtractor#2168) - Add CMake option -DNETWORKING_DEBUG=ON for opt-in debug builds - Replace 3 error messages using printf() with fprintf(stderr): 'Can't send BIN header' 'Can't send BIN data' 'Unable to send data' Error output should go to stderr so it is visible when stdout is redirected (e.g. ccextractor input.ts > output.srt) Fixes CCExtractor#2174
There was a problem hiding this comment.
Pull request overview
This PR addresses #2174 by making networking debug output an explicit CMake opt-in (instead of being always compiled) and by routing networking error messages to stderr.
Changes:
- Replace
DEBUG_OUTinteger toggle with#ifdef NETWORKING_DEBUGinsrc/lib_ccx/networking.c. - Add a
NETWORKING_DEBUGCMake option to enable networking debug output at compile time. - Switch several network send failure messages from
printf()(stdout) tofprintf(stderr, ...)(stderr).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/lib_ccx/networking.c |
Gates debug logging behind NETWORKING_DEBUG and sends error messages to stderr. |
src/CMakeLists.txt |
Introduces a CMake option/define to enable NETWORKING_DEBUG. |
docs/CHANGES.TXT |
Adds a changelog entry for the fixes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
- Fix %u -> %zu for size_t in Sending header and Sending EPG debug logs - Fix %u -> %d for int in Sending bytes debug log - Remove duplicate option(NETWORKING_DEBUG) and if(NETWORKING_DEBUG) blocks in CMakeLists.txt Addresses Copilot review comments on CCExtractor#2175.
CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results, when compared to test for commit f377be9...:
Congratulations: Merging this PR would fix the following tests:
All tests passed completely. Check the result page for more info. |
CCExtractor CI platform finished running the test files on windows. Below is a summary of the test results, when compared to test for commit f377be9...:
Your PR breaks these cases:
It seems that not all tests were passed completely. This is an indication that the output of some files is not as expected (but might be according to you). Check the result page for more info. |
In raising this pull request, I confirm the following (please check boxes):
My familiarity with the project is as follows (check one):
Summary
Fixes #2174 — two related bugs in
src/lib_ccx/networking.c.Fix 1 — Replace
DEBUG_OUTinteger toggle with CMake opt-in#define DEBUG_OUT 0caused debug code to always be compiled intoproduction builds, just silenced at runtime. This is the same pattern
fixed for
VBI_DEBUGin #2168.Removed the
#define DEBUG_OUT 0and replaced all#if DEBUG_OUTguards with
#ifdef NETWORKING_DEBUG, then added a proper CMake option:Developers who need networking debug output can now opt in explicitly:
Fix 2 — Replace
printf()withfprintf(stderr)for error messagesThree error conditions were writing to stdout instead of stderr:
This means network errors are silently swallowed when stdout is
redirected, e.g.
ccextractor input.ts > output.srt. All threehave been changed to
fprintf(stderr, ...), consistent with therest of the error handling in the same file.
Testing
Built and verified locally on macOS with both
NETWORKING_DEBUG=OFF(default) and
NETWORKING_DEBUG=ON. All existing CI tests pass.Fixes #2174