Fix some bug risks and quality issues #243
Conversation
Changes: - Fixed mutable default arguments used in `cve_bin_tool/csv2cve.py` and `cve_bin_tool/cli.py` - Removed unused imports in `cve_bin_tool/cli.py`, `cve_bin_tool/checkers/openssh.py` and `cve_bin_tool/NVDAutoUpdate.py` - Use raw strings in `cve_bin_tool/checkers/openssh.py`, `cve_bin_tool/NVDAutoUpdate.py` and `cve_bin_tool/checkers/expat.py` - Replaced `range(len(..))` with `enumerate()` in `cve_bin_tool/cli.py`
| def main(argv=None, outfile=sys.stdout): | ||
| """ Scan a binary file for certain open source libraries that may have CVEs """ | ||
| if argv is None: | ||
| argv = sys.argv |
There was a problem hiding this comment.
What's the purpose of this change?
There was a problem hiding this comment.
I see your comment says this makes it not mutable, what do you mean by this? My understanding is that this changes preserves mutability. If we wished to make changes to argv not change sys.argv, we'd have to use copy.deepcopy
There was a problem hiding this comment.
This is related to sys.argv being passed as a default argument in the function definition (on line 292).
def main(argv=sys.argv, outfile=sys.stdout):
...It's not recommended to pass mutable objects as default values since the latest passed value is preserved on subsequent function calls, and can cause unintended effects. (ref)
There was a problem hiding this comment.
Ah okay I see what you mean, cool!
| #!/usr/bin/python3 | ||
| # pylint: disable=anomalous-backslash-in-string, invalid-name | ||
| """ | ||
| r""" |
There was a problem hiding this comment.
I don't think we want a r prefix on this docstring
There was a problem hiding this comment.
There's an unescaped backslash in the docstring on line 16. Adding another backslash to escape it can change the meaning, which is why we can convert the docstring to a raw string.
|
Thanks! |
|
@pdxjohnny Replied to the comments. |
3 participants
Changes:
cve_bin_tool/csv2cve.pyandcve_bin_tool/cli.pycve_bin_tool/cli.py,cve_bin_tool/checkers/openssh.pyandcve_bin_tool/NVDAutoUpdate.pycve_bin_tool/checkers/openssh.py,cve_bin_tool/NVDAutoUpdate.pyandcve_bin_tool/checkers/expat.pyrange(len(..))withenumerate()incve_bin_tool/cli.pySome more issues on the repo's DeepSource dashboard on my fork here.