Skip to content

fix(addDummyToLef): prevent fd leaks and add input file validation with clear error handling#4067

Merged
eder-matheus merged 1 commit into
The-OpenROAD-Project:masterfrom
ashnaaseth2325-oss:fix/addDummyToLef-fd-leaks
Mar 30, 2026
Merged

fix(addDummyToLef): prevent fd leaks and add input file validation with clear error handling#4067
eder-matheus merged 1 commit into
The-OpenROAD-Project:masterfrom
ashnaaseth2325-oss:fix/addDummyToLef-fd-leaks

Conversation

@ashnaaseth2325-oss

Copy link
Copy Markdown
Contributor

1. SUMMARY

This PR fixes unsafe file handling in flow/util/addDummyToLef.py by replacing manual open()/close() usage with context managers and adding an input file existence check. It ensures proper resource cleanup and provides clear error messages when the input LEF file is missing.


2. FIX

# Before
f = open(args.inputLef)
content = f.read()
f.close()

# After
if not os.path.isfile(args.inputLef):
    print(f"Error: Input LEF not found: {args.inputLef}", file=sys.stderr)
    sys.exit(1)

with open(args.inputLef) as f:
    content = f.read()

3. VERIFICATION

Run the script with an invalid --inputLef path and confirm it exits cleanly with a clear error message instead of a traceback. Then run it with a valid LEF file and verify the output is generated correctly with no behavior changes. Also confirm no file descriptor issues occur during read/write failures.

@ashnaaseth2325-oss

Copy link
Copy Markdown
Contributor Author

Hi @maliberty @eder-matheus,
Kindly review this PR.
Thanks!

@maliberty maliberty enabled auto-merge March 28, 2026 15:11
Replace bare open()/close() pairs with with-statements and add an
input file existence check with a clear error message.

Signed-off-by: Ashnaa Seth <ashnaaseth2325@gmail.com>
Signed-off-by: ashnaaseth2325-oss <ashnaaseth2325@gmail.com>
auto-merge was automatically disabled March 28, 2026 19:34

Head branch was pushed to by a user without write access

@ashnaaseth2325-oss ashnaaseth2325-oss force-pushed the fix/addDummyToLef-fd-leaks branch from a2c16cd to fa84634 Compare March 28, 2026 19:34
@ashnaaseth2325-oss

Copy link
Copy Markdown
Contributor Author

Hello @maliberty
Thanks for the approval! I pushed a small fix to address the failing checks. CI is running again.
Please feel free to re-enable auto-merge once everything passes.

@ashnaaseth2325-oss

Copy link
Copy Markdown
Contributor Author

Hello @maliberty
Gentle follow-up in case this was missed.

@eder-matheus eder-matheus merged commit 33d1978 into The-OpenROAD-Project:master Mar 30, 2026
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants