fix(discover): make cbm_gitignore_merge atomic on allocation failure#562
Merged
Conversation
cbm_gitignore_merge copied src patterns into dst one strdup at a time and returned void on failure, leaving dst with a partial set of patterns and no signal to the caller. If the dropped pattern was the critical exclude (the .git/info/exclude entry that prevents an OOM walk), the bug could silently recur with some patterns applied and others dropped. Make the merge atomic: on any allocation failure roll back the partial copies so dst is left exactly as it was, and return bool so callers can tell. The .gitignore caller now degrades gracefully (exclude patterns skipped, as if the file were absent) instead of ending up with a half-merged matcher. Adds a reproduce-first test that injects a strdup failure mid-merge via a test seam and asserts dst is unchanged; it is red without the rollback. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.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.
What does this PR do?
Makes
cbm_gitignore_mergeatomic on allocation failure.Previously the merge copied each
srcpattern intodstonestrdupat a time and returnedvoidon failure — leavingdstwith a partial set of patterns and no signal to the caller. If the dropped pattern was the critical exclude (the.git/info/excludeentry that prevents an OOM walk, #489), the matcher could end up half-merged with no indication.Now: on any allocation failure the partial copies are rolled back so
dstis left exactly as it was (atomic), and the function returnsbool. The.gitignorecaller degrades gracefully — the exclude patterns are skipped, as if the file were absent — instead of corrupting the matcher.This was surfaced during a closer review of the (already-merged) #493.
Test
gi_merge_atomic_on_alloc_failureinjects astrdupfailure on the 2ndsrcpattern via a small test seam, then asserts the merge returnsfalseanddstis unchanged (its own pattern still matches; the partially-copied src pattern is gone). It is red without the rollback — the first src pattern leaks intodst.Checklist
git commit -s)