Fix count_oval_object get wrong count #4427
Conversation
oval_root no point to the right oval document root, so get the wrong count.
|
Can one of the admins verify this patch? |
|
@openscap-ci ok to test |
|
@Sep0lkit Nice catch! Thanks for the fix. The objects defined in definitions which are referenced only via See for example rule Would you be interested in fixing this issue too? |
|
@yuumasato understand your view, I will try to fix this issue. |
|
ITOH, if a definition is extended by multiple definitions, the OVAL count will be bloated up. |
count extend_definition objects
resolve pep 8 issues
There was a problem hiding this comment.
@Sep0lkit Thanks for updating the PR, and sorry for delay in review.
The script will now list the all OVAL object types used in each rule correctly.
But I have noticed that the numbers listed at thae end are not actually the number of times an object is used/instantiated, but the number of times a rule used an object of that type.
The summary title of Count of used OVAL objects is misleading, could you updated it to Count of rules using the object.
| for extend_def in definition.findall(".//extend_definition"): | ||
| extend_ref = extend_def.attrib["definition_ref"] | ||
| t = get_ext_def_tests(oval_root, extend_ref) | ||
| tests += t |
There was a problem hiding this comment.
As far as I know, there shouldn't be difference under the hood, but lets be consistent and use tests.extend()
| root = it.root | ||
| return root | ||
| except: | ||
| sys.stderr.write("Error while loading file " + file_name + ".\n") | ||
| exit(-1) | ||
|
|
||
|
|
||
| def get_ext_def_tests(oval_root, def_refs): | ||
| t = [] |
There was a problem hiding this comment.
Please name the variable clearly, for example extended_tests.
| for criterion in definition.findall(".//criterion"): | ||
| test_ref = criterion.attrib["test_ref"] | ||
| tests.append(test_ref) | ||
| for extend_def in definition.findall(".//extend_definition"): | ||
| extend_ref = extend_def.attrib["definition_ref"] | ||
| t = get_ext_def_tests(oval_root, extend_ref) |
There was a problem hiding this comment.
Here as well, name the variable more significantly.
|
Hello @Sep0lkit, there are some small suggestions to your PR, so we would love to see more commits from you. Do you still plan to pursue the merge? |
Description:
Rationale: