| Message ID | 20221215213429.998948-4-jwcart2@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Petr Lautrbach |
| Headers | show |
| Series | Add CIL Deny Rule | expand |
On 12/15/2022 4:34 PM, James Carter wrote: > Add the function cil_tree_remove_node() which takes a node pointer > as an input, finds the parent, walks the list of nodes to the node > prior to the given node, and then updates that nodes next pointer > to remove the given node from the tree. > > Signed-off-by: James Carter <jwcart2@gmail.com> > --- > libsepol/cil/src/cil_tree.c | 27 +++++++++++++++++++++++++++ > libsepol/cil/src/cil_tree.h | 1 + > 2 files changed, 28 insertions(+) > > diff --git a/libsepol/cil/src/cil_tree.c b/libsepol/cil/src/cil_tree.c > index 6376c208..73b4e135 100644 > --- a/libsepol/cil/src/cil_tree.c > +++ b/libsepol/cil/src/cil_tree.c > @@ -248,6 +248,33 @@ void cil_tree_node_destroy(struct cil_tree_node **node) > *node = NULL; > } > > +void cil_tree_remove_node(struct cil_tree_node *node) > +{ > + struct cil_tree_node *parent, *curr; > + > + if (node == NULL || node->parent == NULL) { > + return; > + } > + > + parent = node->parent; > + > + if (parent->cl_head == node) { > + parent->cl_head = node->next; > + return; > + } > + > + curr = parent->cl_head; > + while (curr && curr->next != node) { > + curr = curr->next; > + } > + > + if (curr == NULL) { > + return; > + } > + > + curr->next = node->next; > +} > + Is there a reason this leaves it to the caller to call cil_tree_node_destroy()? It just feels a little weird to leave the node unreachable without freeing. It looks like both callers in the series (cil_process_deny_rule(s)) call cil_tree_node_destroy() immediately after. -Daniel > /* Perform depth-first walk of the tree > Parameters: > start_node: root node to start walking from > diff --git a/libsepol/cil/src/cil_tree.h b/libsepol/cil/src/cil_tree.h > index 5a98da55..e4b3fd04 100644 > --- a/libsepol/cil/src/cil_tree.h > +++ b/libsepol/cil/src/cil_tree.h > @@ -63,6 +63,7 @@ void cil_tree_children_destroy(struct cil_tree_node *node); > > void cil_tree_node_init(struct cil_tree_node **node); > void cil_tree_node_destroy(struct cil_tree_node **node); > +void cil_tree_remove_node(struct cil_tree_node *node); > > //finished values > #define CIL_TREE_SKIP_NOTHING 0
On Fri, Feb 3, 2023 at 5:54 PM Daniel Burgener <dburgener@linux.microsoft.com> wrote: > > On 12/15/2022 4:34 PM, James Carter wrote: > > Add the function cil_tree_remove_node() which takes a node pointer > > as an input, finds the parent, walks the list of nodes to the node > > prior to the given node, and then updates that nodes next pointer > > to remove the given node from the tree. > > > > Signed-off-by: James Carter <jwcart2@gmail.com> > > --- > > libsepol/cil/src/cil_tree.c | 27 +++++++++++++++++++++++++++ > > libsepol/cil/src/cil_tree.h | 1 + > > 2 files changed, 28 insertions(+) > > > > diff --git a/libsepol/cil/src/cil_tree.c b/libsepol/cil/src/cil_tree.c > > index 6376c208..73b4e135 100644 > > --- a/libsepol/cil/src/cil_tree.c > > +++ b/libsepol/cil/src/cil_tree.c > > @@ -248,6 +248,33 @@ void cil_tree_node_destroy(struct cil_tree_node **node) > > *node = NULL; > > } > > > > +void cil_tree_remove_node(struct cil_tree_node *node) > > +{ > > + struct cil_tree_node *parent, *curr; > > + > > + if (node == NULL || node->parent == NULL) { > > + return; > > + } > > + > > + parent = node->parent; > > + > > + if (parent->cl_head == node) { > > + parent->cl_head = node->next; > > + return; > > + } > > + > > + curr = parent->cl_head; > > + while (curr && curr->next != node) { > > + curr = curr->next; > > + } > > + > > + if (curr == NULL) { > > + return; > > + } > > + > > + curr->next = node->next; > > +} > > + > > Is there a reason this leaves it to the caller to call > cil_tree_node_destroy()? It just feels a little weird to leave the node > unreachable without freeing. It looks like both callers in the series > (cil_process_deny_rule(s)) call cil_tree_node_destroy() immediately after. > > -Daniel > Not that I can remember. I can't really think of a scenario where I would want to remove a node, but not destroy it. It also seems like it would be better to name it cil_tree_node_remove() to fit the naming scheme of the other functions. Thanks, Jim > > /* Perform depth-first walk of the tree > > Parameters: > > start_node: root node to start walking from > > diff --git a/libsepol/cil/src/cil_tree.h b/libsepol/cil/src/cil_tree.h > > index 5a98da55..e4b3fd04 100644 > > --- a/libsepol/cil/src/cil_tree.h > > +++ b/libsepol/cil/src/cil_tree.h > > @@ -63,6 +63,7 @@ void cil_tree_children_destroy(struct cil_tree_node *node); > > > > void cil_tree_node_init(struct cil_tree_node **node); > > void cil_tree_node_destroy(struct cil_tree_node **node); > > +void cil_tree_remove_node(struct cil_tree_node *node); > > > > //finished values > > #define CIL_TREE_SKIP_NOTHING 0 >
diff --git a/libsepol/cil/src/cil_tree.c b/libsepol/cil/src/cil_tree.c index 6376c208..73b4e135 100644 --- a/libsepol/cil/src/cil_tree.c +++ b/libsepol/cil/src/cil_tree.c @@ -248,6 +248,33 @@ void cil_tree_node_destroy(struct cil_tree_node **node) *node = NULL; } +void cil_tree_remove_node(struct cil_tree_node *node) +{ + struct cil_tree_node *parent, *curr; + + if (node == NULL || node->parent == NULL) { + return; + } + + parent = node->parent; + + if (parent->cl_head == node) { + parent->cl_head = node->next; + return; + } + + curr = parent->cl_head; + while (curr && curr->next != node) { + curr = curr->next; + } + + if (curr == NULL) { + return; + } + + curr->next = node->next; +} + /* Perform depth-first walk of the tree Parameters: start_node: root node to start walking from diff --git a/libsepol/cil/src/cil_tree.h b/libsepol/cil/src/cil_tree.h index 5a98da55..e4b3fd04 100644 --- a/libsepol/cil/src/cil_tree.h +++ b/libsepol/cil/src/cil_tree.h @@ -63,6 +63,7 @@ void cil_tree_children_destroy(struct cil_tree_node *node); void cil_tree_node_init(struct cil_tree_node **node); void cil_tree_node_destroy(struct cil_tree_node **node); +void cil_tree_remove_node(struct cil_tree_node *node); //finished values #define CIL_TREE_SKIP_NOTHING 0
Add the function cil_tree_remove_node() which takes a node pointer as an input, finds the parent, walks the list of nodes to the node prior to the given node, and then updates that nodes next pointer to remove the given node from the tree. Signed-off-by: James Carter <jwcart2@gmail.com> --- libsepol/cil/src/cil_tree.c | 27 +++++++++++++++++++++++++++ libsepol/cil/src/cil_tree.h | 1 + 2 files changed, 28 insertions(+)