From 869a97aa259dffa2620dabcad84e1c22545ffc3d Mon Sep 17 00:00:00 2001
From: Daiki Ueno <ueno@gnu.org>
Date: Fri, 8 Nov 2024 16:05:32 +0900
Subject: [PATCH] asn1_find_node: optimize "?NUMBER" node lookup with indexing

To avoid linear search of named nodes, this adds a array of child
nodes to their parent nodes as a cache.

Signed-off-by: Daiki Ueno <ueno@gnu.org>
Signed-off-by: Simon Josefsson <simon@josefsson.org>
---
 lib/element.c    | 56 ++++++++++++++++++++++++++++++++++++++++++------
 lib/element.h    | 10 +++++++++
 lib/int.h        |  8 +++++++
 lib/parser_aux.c | 10 +++++++++
 lib/structure.c  | 13 +++++++++++
 5 files changed, 90 insertions(+), 7 deletions(-)

diff --git a/lib/element.c b/lib/element.c
index 850bef4a..528df418 100644
--- a/lib/element.c
+++ b/lib/element.c
@@ -33,6 +33,8 @@
 #include "structure.h"
 #include "c-ctype.h"
 #include "element.h"
+#include <limits.h>
+#include "intprops.h"
 
 void
 _asn1_hierarchical_name (asn1_node_const node, char *name, int name_size)
@@ -129,6 +131,41 @@ _asn1_convert_integer (const unsigned char *value, unsigned char *value_out,
   return ASN1_SUCCESS;
 }
 
+int
+_asn1_node_array_set (struct asn1_node_array_st *array, size_t position,
+		      asn1_node node)
+{
+  if (position >= array->size)
+    {
+      size_t new_size = position, i;
+      asn1_node *new_nodes;
+
+      if (INT_MULTIPLY_OVERFLOW (new_size, 2))
+	return ASN1_GENERIC_ERROR;
+      new_size *= 2;
+
+      if (INT_ADD_OVERFLOW (new_size, 1))
+	return ASN1_GENERIC_ERROR;
+      new_size += 1;
+
+      if (INT_MULTIPLY_OVERFLOW (new_size, sizeof (*new_nodes)))
+	return ASN1_GENERIC_ERROR;
+
+      new_nodes = realloc (array->nodes, new_size * sizeof (*new_nodes));
+      if (!new_nodes)
+	return ASN1_MEM_ALLOC_ERROR;
+
+      for (i = array->size; i < new_size; i++)
+	new_nodes[i] = NULL;
+
+      array->nodes = new_nodes;
+      array->size = new_size;
+    }
+
+  array->nodes[position] = node;
+  return ASN1_SUCCESS;
+}
+
 /* Appends a new element into the sequence (or set) defined by this
  * node. The new element will have a name of '?number', where number
  * is a monotonically increased serial number.
@@ -145,6 +182,7 @@ _asn1_append_sequence_set (asn1_node node, struct node_tail_cache_st *pcache)
   asn1_node p, p2;
   char temp[LTOSTR_MAX_SIZE + 1];
   long n;
+  int result;
 
   if (!node || !(node->down))
     return ASN1_GENERIC_ERROR;
@@ -177,17 +215,21 @@ _asn1_append_sequence_set (asn1_node node, struct node_tail_cache_st *pcache)
       pcache->tail = p2;
     }
 
-  if (p->name[0] == 0)
-    _asn1_str_cpy (temp, sizeof (temp), "?1");
-  else
+  n = 0;
+  if (p->name[0] != 0)
     {
-      n = strtol (p->name + 1, NULL, 0);
-      n++;
-      temp[0] = '?';
-      _asn1_ltostr (n, temp + 1);
+      n = strtol (p->name + 1, NULL, 10);
+      if (n <= 0 || n >= LONG_MAX - 1)
+	return ASN1_GENERIC_ERROR;
     }
+  temp[0] = '?';
+  _asn1_ltostr (n + 1, temp + 1);
   _asn1_set_name (p2, temp);
   /*  p2->type |= CONST_OPTION; */
+  result = _asn1_node_array_set (&node->numbered_children, n, p2);
+  if (result != ASN1_SUCCESS)
+    return result;
+  p2->parent = node;
 
   return ASN1_SUCCESS;
 }
diff --git a/lib/element.h b/lib/element.h
index 732054e9..b84e3a27 100644
--- a/lib/element.h
+++ b/lib/element.h
@@ -38,4 +38,14 @@ int _asn1_convert_integer (const unsigned char *value,
 void _asn1_hierarchical_name (asn1_node_const node, char *name,
 			      int name_size);
 
+static inline asn1_node_const
+_asn1_node_array_get (const struct asn1_node_array_st *array, size_t position)
+{
+  return position < array->size ? array->nodes[position] : NULL;
+}
+
+int
+_asn1_node_array_set (struct asn1_node_array_st *array, size_t position,
+		      asn1_node node);
+
 #endif
diff --git a/lib/int.h b/lib/int.h
index 4f2d98d1..41b12b0b 100644
--- a/lib/int.h
+++ b/lib/int.h
@@ -31,6 +31,12 @@
 
 # define ASN1_SMALL_VALUE_SIZE 16
 
+struct asn1_node_array_st
+{
+  asn1_node *nodes;
+  size_t size;
+};
+
 /* This structure is also in libtasn1.h, but then contains less
    fields.  You cannot make any modifications to these first fields
    without breaking ABI.  */
@@ -47,6 +53,8 @@ struct asn1_node_st
   asn1_node left;		/* Pointer to the next list element */
   /* private fields: */
   unsigned char small_value[ASN1_SMALL_VALUE_SIZE];	/* For small values */
+  asn1_node parent;		/* Pointer to the parent node */
+  struct asn1_node_array_st numbered_children; /* Array of unnamed child nodes for caching */
 
   /* values used during decoding/coding */
   int tmp_ival;
diff --git a/lib/parser_aux.c b/lib/parser_aux.c
index 415905a0..4281cc97 100644
--- a/lib/parser_aux.c
+++ b/lib/parser_aux.c
@@ -126,6 +126,7 @@ asn1_find_node (asn1_node_const pointer, const char *name)
   const char *n_start;
   unsigned int nsize;
   unsigned int nhash;
+  const struct asn1_node_array_st *numbered_children;
 
   if (pointer == NULL)
     return NULL;
@@ -209,6 +210,7 @@ asn1_find_node (asn1_node_const pointer, const char *name)
       if (p->down == NULL)
 	return NULL;
 
+      numbered_children = &p->numbered_children;
       p = p->down;
       if (p == NULL)
 	return NULL;
@@ -222,6 +224,12 @@ asn1_find_node (asn1_node_const pointer, const char *name)
 	}
       else
 	{			/* no "?LAST" */
+	  if (n[0] == '?' && c_isdigit (n[1]))
+	    {
+	      long position = strtol (n + 1, NULL, 10);
+	      if (position > 0 && position < LONG_MAX)
+		p = _asn1_node_array_get (numbered_children, position - 1);
+	    }
 	  while (p)
 	    {
 	      if (p->name_hash == nhash && !strcmp (p->name, n))
@@ -509,6 +517,8 @@ _asn1_remove_node (asn1_node node, unsigned int flags)
       if (node->value != node->small_value)
 	free (node->value);
     }
+
+  free (node->numbered_children.nodes);
   free (node);
 }
 
diff --git a/lib/structure.c b/lib/structure.c
index 9c95b9e2..32692ad2 100644
--- a/lib/structure.c
+++ b/lib/structure.c
@@ -31,6 +31,9 @@
 #include <structure.h>
 #include "parser_aux.h"
 #include <gstr.h>
+#include "c-ctype.h"
+#include "element.h"
+#include <limits.h>
 
 
 extern char _asn1_identifierMissing[];
@@ -391,6 +394,16 @@ asn1_delete_element (asn1_node structure, const char *element_name)
   if (source_node == NULL)
     return ASN1_ELEMENT_NOT_FOUND;
 
+  if (source_node->parent
+      && source_node->name[0] == '?'
+      && c_isdigit (source_node->name[1]))
+    {
+      long position = strtol (source_node->name + 1, NULL, 10);
+      if (position > 0 && position < LONG_MAX)
+	_asn1_node_array_set (&source_node->parent->numbered_children,
+			      position - 1, NULL);
+    }
+
   p2 = source_node->right;
   p3 = _asn1_find_left (source_node);
   if (!p3)
-- 
GitLab