On 9/27/2023 4:41 PM, James Carter wrote:
On Wed, Sep 27, 2023 at 3:27 PM Daniel Burgener
<dburgener@xxxxxxxxxxxxxxxxxxx> wrote:
@@ -3661,21 +3615,17 @@ static int cil_check_for_bad_inheritance(struct cil_tree_node *node)
return rc;
}
-static int __cil_resolve_ast_node(struct cil_tree_node *node, void *extra_args)
+static int __cil_resolve_ast_node(struct cil_tree_node *node, struct cil_args_resolve *args)
{
int rc = SEPOL_OK;
- struct cil_args_resolve *args = extra_args;
+ struct cil_db *db = args->db;
enum cil_pass pass = 0;
- if (node == NULL || args == NULL) {
- goto exit;
- }
-
Is deleting the "node == NULL" part of this check intended here? It
seems unrelated to the rest of the commit, and it's not locally obvious
that it's safe.
You are right. It is not related to the rest of the commit. There are
a bunch of these sorts of checks that are useless and really annoy me.
The function __cil_resolve_ast_node() is called once from
__cil_resolve_ast_node_helper() and neither node nor args can be NULL.
Since I was changing something nearby, I guess I couldn't resist. I
can leave it in, if people prefer. It doesn't cause any harm, other
than annoying me.
As is is fine by me. Your explanation makes sense. I mostly wanted to
make sure it was reasoned out rather than an accidental drop, but now
that you point it out, it does look impossible for this to be NULL.
Reviewed-by: Daniel Burgener <dburgener@xxxxxxxxxxxxxxxxxxx>
