Antoine Pitrou <pit...@free.fr> added the comment: Patch 0002:
- cached_info->error_msg doesn't seem deallocated anywhere? Patch 0003: - "usedforsecurity" is a poor name IMO; make it shorter and/or PEP8-ize it ("used_for_security") - the 2-element context array thing is obscure: why not distinct "ctx" and "ctx_non_fips" members? - "this could fail, e.g. low on memory, or encodings": doesn't it lack an error-handling path, then? Patch 0004: - openssl_can_enforce_fips(): instead of calling OpenSSL in a subprocess, perhaps it's possible to expose a public flag in the hashlib module (e.g. "hashlib.HAS_FIPS")? or is this info not fetchable programmatically? - openssl_can_enforce_fips() needs to check the subprocess return code, in case another error happened - run_command_with_fips_enforcement() should use the assert_python_ok() and assert_python_failure() functions from Lib/test/script_helper.py Overall: - please put back the unconditional tests for the "usedforsecurity" argument (even when FIPS can't be enforced) - the patches lack docs (Doc/library/hashlib.rst) - please commit all this as a single commit, not 4 different ones ---------- nosy: +pitrou _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue9216> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com