Repository navigation
Fix unclosed file handles in Converter.parse_graminit_h/_c - #5432
Merged
Merged
Conversation
Both methods opened the graminit .h/.c file with a bare open() call and read it (line-by-line and via next()) for the rest of the function, without ever closing it - no with block, no matching close(), and no finally. Read each file's lines up front inside a with block, then iterate over the resulting list instead of the file object directly: parse_graminit_h continues iterating with a plain for loop, parse_graminit_c wraps the list in iter() to preserve the next(f) calls it depends on throughout the rest of the function unchanged. Verified with a synthetic pgen-output fixture exercising both methods end to end (success path matches the original line-by-line parse exactly, including all the same symbol/dfa/label/grammar assertions; error path on a missing file still returns False) and a descriptor-count check confirming the handle is actually released after a successful parse.
cobaltt7
added
the
ci: skip news
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
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
blib2to3.pgen2.conv.Converter.parse_graminit_hand.parse_graminit_ceach open the grammar.h/.cfile with a baref = open(filename)and then read fromffor the rest of the function (afor line in f:loop in the_hcase, directnext(f)calls throughout the_ccase). Neither method ever closes the handle — nowith, no matching.close(), nofinally.Found via codehound, a static analyzer I built; this repo's copy of
blib2to3was part of a broader scan.Fix
Read the file's lines up front inside a
withblock, then operate on the resulting list instead of the file object directly:parse_graminit_h:for line in f→for line in lines, wherelinescomes fromwith open(filename) as f: lines = f.readlines().parse_graminit_c: same idea, but wrapped initer(...)since the rest of the function drives itself entirely throughnext(f)calls (the existing comment even calls this out:# The code below essentially uses f's iterator-ness!) —f = iter(fh.readlines())preserves that unchanged.Both changes are scoped to the
open(...)line and the following loop header only; the parsing logic itself (all the regex matching, asserts, and table-building in the ~160-lineparse_graminit_cbody) is untouched, so the diff stays minimal per the norm from #3363/#3794.Testing
Converterisn't currently exercised by any test in this repo (nothing else in the tree imports it — it looks like a standalone pgen-output-loading utility), so I built a synthetic pgen-format.h/.cfixture pair and drove both methods through it directly:parse_graminit_h/parse_graminit_cboth reproduce the exact samesymbol2number/number2symbol/dfas/labels/states/startresults as before the change, for a minimal one-symbol grammar.Falseand prints the same message.Also ran
python -c "import blib2to3.pgen2.conv"to confirm no syntax/import issues.