Fix resource leaks and unsafe shell commands in Python scripts #2722
Conversation
- Refactored `SU2/io/config.py` to use `try...finally` blocks for file handling, ensuring proper resource cleanup. - Replaced unsafe `subprocess.Popen` calls for creating symbolic links in `SU2/eval/functions.py` and `SU2/eval/gradients.py` with `os.symlink`. - Added checks for existing links before creation to prevent errors. Signed-off-by: shbhmexe <shubhushukla586@gmail.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if not this_con: | ||
| continue # if no definition | ||
| # defaults | ||
| this_obj = "NONE" |
Check warning
Code scanning / CodeQL
Variable defined multiple times Warning
| this_obj = "NONE" | ||
| this_sgn = "=" | ||
| this_scl = 1.0 | ||
| this_val = 0.0 |
Check warning
Code scanning / CodeQL
Variable defined multiple times Warning
| # split across equals sign | ||
| line = line.split("=") | ||
| this_param = line[0].strip() | ||
| old_value = line[1].strip() |
Check notice
Code scanning / CodeQL
Unused local variable Note
| if "TARGET_CL" not in data_dict: | ||
| data_dict["TARGET_CL"] = 0.0 | ||
| if "FREESTREAM_PRESSURE" not in data_dict: | ||
| data_dict["FREESTREAM_PRESSURE"] = 101325.0 |
Check warning
Code scanning / CodeQL
Variable defined multiple times Warning
|
? |
|
Buddy I told you before we are not machines. |
Apologies, sir. No passive aggression intended. Thank you for reviewing and approving the PR. |
2 participants
This PR addresses critical resource leaks and unsafe coding practices in the Python wrapper scripts:
SU2_PY/SU2/io/config.py, file operations inread_config,write_config, anddump_configwere opening files without closing them. These have been refactored to usetry...finallyblocks or context managers (with open(...)) to ensure files are closed properly, preventing file descriptor exhaustion.SU2_PY/SU2/eval/functions.pyandSU2_PY/SU2/eval/gradients.py, symbolic links were being created usingsubprocess.Popen(['ln', '-s', ...]). This is platform-dependent and unreliable. These have been replaced with the portableos.symlinkfunction, including checks for existing links.Related Work
PR Checklist
Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.
pre-commit run --allto format old commits.