Fix a variety of problems #4
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes critical issues that prevented the code from running correctly, primarily addressing module-level execution problems and command construction issues.
- Wraps module-level code in
evaluate_rsr.pywith propermain()function andif __name__ == "__main__":guard to prevent execution on import - Fixes command construction in
compile_ossfuzz.pyby changing from shell string concatenation to proper list-based arguments for safer execution - Makes
--decompiled-dataseta required argument and improves error messaging
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| evaluate_rsr.py | Refactored module-level code into parse_args() and main() functions with proper execution guard, preventing unintended execution when the module is imported by other scripts |
| compile_ossfuzz.py | Fixed command construction by converting extra_flags to a list and using shell=False for safer subprocess execution; corrected Docker path mapping and fixed multiprocessing with functools.partial |
Comments suppressed due to low confidence (1)
compile_ossfuzz.py:204
- Variable out is not used.
out = container.exec_in_container(
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not args.decompiled_dataset: | ||
| raise ValueError( | ||
| "--decompiled-dataset is required. Please provide the path to the merged dataset.") | ||
|
|
There was a problem hiding this comment.
This validation check is redundant because the argument parser already has required=True for the --decompiled-dataset argument (line 30). The argparse module will raise an error if this argument is not provided, so this condition will never be true.
| if not args.decompiled_dataset: | |
| raise ValueError( | |
| "--decompiled-dataset is required. Please provide the path to the merged dataset.") |
| container.exec_in_container( | ||
| cmd, cwd='/challenges', shell=True, check=True) | ||
| '-o', str(output_file_indocker)] + extra_flags + ['-lm'] | ||
| out = container.exec_in_container( |
There was a problem hiding this comment.
The variable out is assigned but never used. If the intention is to capture and suppress the output from the Docker container execution, consider either using the result (e.g., for logging or debugging) or removing the assignment and just calling the function directly.
| out = container.exec_in_container( | |
| container.exec_in_container( |
1 participant
I ran into a variety of problems when trying to run the code in the repository. This PR fixes most of them.