fix: stop recode file crashing with a NULL codefrom - #160
Open
antLI-dev wants to merge 1 commit into
Open
Conversation
"recode file <recodefile>" fully delegates to read_station_recode_file, which already applies every recode from the file itself. The function never returned afterward though, so execution fell through into logic meant only for the other two recode grammars (inline "recode X to Y" and "recode suffix ..."), which share optional uncertainty/date-range parsing this form has no equivalent of. That logic ends with a call to add_stn_recode_to_map_err using codefrom/codeto, which are never set on the file branch - codefrom stays NULL, and _stricmp(codefrom, ...) segfaults immediately. This meant every use of "recode file X" crashed unconditionally, with an empty .err file since the crash happens before close_output_files() can run - not a fault in the recode file's own content. Reproduced and confirmed fixed with AddressSanitizer, pointing directly at the NULL dereference. Full regression suite (testall.pl -r) passes unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.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
1 participant
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.
Summary
Every use of
recode file <recodefile>crashed SNAP unconditionally,regardless of the recode file's own content, leaving an empty
.errfilebehind - matching a user report of the feature being "unclear... doesn't
work... error file empty". Reported by @mermy
read_station_recode_definition(stnrecode.cpp:410) handles threedistinct recode grammars in one function:
file,suffix, and inline<code> to <code>. Thefilebranch fully delegates toread_station_recode_file, which already applies every recode from theCSV itself - but the branch never returned afterward, so execution fell
through into ~200 lines of optional uncertainty/date-range parsing that
only apply to the other two grammars (confirmed: the CSV format's own
columns have no uncertainty equivalent at all), ending in a call to
add_stn_recode_to_map_errwithcodefrom/codetostillNULL(neverset on this branch).
_stricmp(NULL, ...)segfaults immediately.The crash happening before
close_output_files()can run is why the.errfile was empty - same underlying mechanism asfix/snap-exit-code-error-reporting, just unreachable by that fix since a
segfault isn't a C++ exception.
Fix: the
filebranch now returns immediately after handling itself,since it never shared any of the fall-through logic to begin with.
Test plan
.snp/.crd/.dat/recode-CSV, release build segfaults immediately, both.errand
.lstcome out as 0 bytes)gdbavailable, no root toinstall it) - pointed directly at the NULL dereference
a correct
.lstwith the recode actually applied, no.errtestall.pl -r) passes unchangedGSR-1011