diff mbox series

[nft,1/4] segtree: Drop needless insertion in ei_insert()

Message ID 20200123143049.13888-2-phil@nwl.cc
State Changes Requested
Delegated to: Pablo Neira
Headers show
Series Covscan-induced review of ei_insert() | expand

Commit Message

Phil Sutter Jan. 23, 2020, 2:30 p.m. UTC 
Code checks whether for two new ranges one fully includes the other. If
so, it would add the contained one only for segtree_linearize() to later
omit the redundant items.

Instead just drop the contained item (which will always come last
because caller orders the new elements in beforehand).

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/segtree.c | 25 ++-----------------------
 1 file changed, 2 insertions(+), 23 deletions(-)

Comments

Pablo Neira Ayuso Jan. 28, 2020, 11:22 a.m. UTC | #1 
On Thu, Jan 23, 2020 at 03:30:46PM +0100, Phil Sutter wrote:
> Code checks whether for two new ranges one fully includes the other. If
> so, it would add the contained one only for segtree_linearize() to later
> omit the redundant items.
> 
> Instead just drop the contained item (which will always come last
> because caller orders the new elements in beforehand).
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>

I would probably append this to the patch description.

* The auto-merge feature for sets merges what it has just been split
  by this code thereafter, so it turns this code into no-op.

* The auto-merge feature is not available for maps at this stage.
  This code allows to split intervals that have a different rhs mapping.
  This could be used in that case, but I find this feature confusing
  userwise.

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
diff mbox series

Patch

diff --git a/src/segtree.c b/src/segtree.c
index e8e32412f3a41..aa1f1c38d789c 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -191,9 +191,6 @@  static int ei_insert(struct list_head *msgs, struct seg_tree *tree,
 		     struct elementary_interval *new, bool merge)
 {
 	struct elementary_interval *lei, *rei;
-	mpz_t p;
-
-	mpz_init2(p, tree->keylen);
 
 	/*
 	 * Lookup the intervals containing the left and right endpoints.
@@ -207,25 +204,9 @@  static int ei_insert(struct list_head *msgs, struct seg_tree *tree,
 	if (lei != NULL && rei != NULL && lei == rei) {
 		if (!merge)
 			goto err;
-		/*
-		 * The new interval is entirely contained in the same interval,
-		 * split it into two parts:
-		 *
-		 * [lei_left, new_left) and (new_right, rei_right]
-		 */
-		if (segtree_debug(tree->debug_mask))
-			pr_gmp_debug("split [%Zx %Zx]\n", lei->left, lei->right);
 
-		ei_remove(tree, lei);
-
-		mpz_sub_ui(p, new->left, 1);
-		if (mpz_cmp(lei->left, p) <= 0)
-			__ei_insert(tree, ei_alloc(lei->left, p, lei->expr, 0));
-
-		mpz_add_ui(p, new->right, 1);
-		if (mpz_cmp(p, rei->right) < 0)
-			__ei_insert(tree, ei_alloc(p, rei->right, lei->expr, 0));
-		ei_destroy(lei);
+		ei_destroy(new);
+		return 0;
 	} else {
 		if (lei != NULL) {
 			if (!merge)
@@ -271,8 +252,6 @@  static int ei_insert(struct list_head *msgs, struct seg_tree *tree,
 
 	__ei_insert(tree, new);
 
-	mpz_clear(p);
-
 	return 0;
 err:
 	errno = EEXIST;