From 012158fb241dcf8ef636119820dec8418be80134 Mon Sep 17 00:00:00 2001
From: Olof hagsand <olof@hagsand.se>
Date: Sun, 10 Dec 2023 21:03:20 +0100
Subject: [PATCH] Optimization of yang_find()

---
 lib/clixon/clixon_yang.h        |  6 ++---
 lib/src/clixon_datastore_read.c |  8 +++----
 lib/src/clixon_yang.c           | 39 ++++++++++++++++-----------------
 lib/src/clixon_yang_internal.h  |  1 -
 4 files changed, 25 insertions(+), 29 deletions(-)

diff --git a/lib/clixon/clixon_yang.h b/lib/clixon/clixon_yang.h
index 5a0399c6..e1062596 100644
--- a/lib/clixon/clixon_yang.h
+++ b/lib/clixon/clixon_yang.h
@@ -83,9 +83,9 @@
                                       *     leaf z;
                                       *  }
                                       */
-#define YANG_FLAG_MOUNTPOINT   0x100 /* Mark node as populated mount-point
-                                      * Set by yang_mount_set.
-                                      * Used by ys_free
+#define YANG_FLAG_MOUNTPOINT 0x100  /* Mark node as ACTUAL populated mount-point
+                                      * Set by yang_mount_set 
+                                      * Read by ys_free1
                                       */
 
 /*
diff --git a/lib/src/clixon_datastore_read.c b/lib/src/clixon_datastore_read.c
index fb9d5765..4d10dd78 100644
--- a/lib/src/clixon_datastore_read.c
+++ b/lib/src/clixon_datastore_read.c
@@ -948,13 +948,11 @@ xmldb_get_cache(clicon_handle     h,
     } /* x0t == NULL */
     else
         x0t = de->de_xml;
-
-    if (yb == YB_MODULE && !xml_spec(x0t)){
+    if (yb == YB_MODULE && !xml_spec(x0t)){ 
+        /* Seems unneccesary to always do this, but breaks test_plugin_reset.sh if removed */
         if ((ret = xml_bind_yang(h, x0t, YB_MODULE, yspec, xerr)) < 0)
             goto done;
-        if (ret == 0)
-            ; /* XXX */
-        else {
+        if (ret == 1) {
             /* Add default global values (to make xpath below include defaults) */
             if (xml_global_defaults(h, x0t, nsc, xpath, yspec, 0) < 0)
                 goto done;
diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c
index 1d7971f1..26ef7f38 100644
--- a/lib/src/clixon_yang.c
+++ b/lib/src/clixon_yang.c
@@ -1108,12 +1108,14 @@ yn_each(yang_stmt *yparent,
 
 /*! Find first child yang_stmt with matching keyword and argument
  *
+ * Find child given keyword and argument.
+ * Special case: look in imported INPUTs as well (for (sub)modules.
+ * This could need an optimized lookup, especially the INPUT case.
+ * Most common use for the special case, ie in openconfig, is grouping and identity
  * @param[in]  yn         Yang node, current context node.
  * @param[in]  keyword    if 0 match any keyword. Actual type: enum rfc_6020
  * @param[in]  argument   String compare w argument. if NULL, match any.
  * @retval     ys         Yang statement, if any
- * This however means that if you actually want to match only a yang-stmt with 
- * argument==NULL you cannot, but I have not seen any such examples.
  * @see yang_find_datanode
  * @see yang_match  returns number of matches
  */
@@ -1125,10 +1127,11 @@ yang_find(yang_stmt  *yn,
     yang_stmt *ys = NULL;
     int        i;
     yang_stmt *yret = NULL;
+    yang_stmt *yretsub = NULL;
     char      *name;
     yang_stmt *yspec;
     yang_stmt *ym;
-
+    
     for (i=0; i<yn->ys_len; i++){
         ys = yn->ys_stmt[i];
         if (keyword == 0 || ys->ys_keyword == keyword){
@@ -1138,24 +1141,22 @@ yang_find(yang_stmt  *yn,
                 break;
             }
         }
-    }
-    /* Special case: if not match and yang node is module or submodule, extend
-     * search to include submodules */
-    if (yret == NULL &&
-        (yang_keyword_get(yn) == Y_MODULE ||
-         yang_keyword_get(yn) == Y_SUBMODULE)){
-        yspec = ys_spec(yn);
-        for (i=0; i<yn->ys_len; i++){
-            ys = yn->ys_stmt[i];
-            if (yang_keyword_get(ys) == Y_INCLUDE){
-                name = yang_argument_get(ys);
-                if ((ym = yang_find_module_by_name(yspec, name)) != NULL &&
-                    (yret = yang_find(ym, keyword, argument)) != NULL)
-                    break;
+        /* Special case: if not match and yang node is module or submodule, extend
+         * search to include submodules 
+         */
+        if (yretsub == NULL &&
+            yang_keyword_get(ys) == Y_INCLUDE &&
+            keyword != Y_NAMESPACE &&
+            (yang_keyword_get(yn) == Y_MODULE ||
+             yang_keyword_get(yn) == Y_SUBMODULE)){
+            yspec = ys_spec(yn);
+            name = yang_argument_get(ys);
+            if ((ym = yang_find_module_by_name(yspec, name)) != NULL) {
+                yretsub = yang_find(ym, keyword, argument);
             }
         }
     }
-    return yret;
+    return yret?yret:yretsub;
 }
 
 /*! Count number of children that matches keyword and argument
@@ -3863,7 +3864,6 @@ yang_anydata_add(yang_stmt *yp,
  *     }
  * @endcode
  * @see ys_populate_unknown  Called when parsing YANG
- * XXX consider optimizing, the call to yang_find_prefix_by_namespace may be slow
  */
 int
 yang_extension_value(yang_stmt *ys,
@@ -3896,7 +3896,6 @@ yang_extension_value(yang_stmt *ys,
             continue;
         if ((ymod = ys_module(yext)) == NULL)
             continue;
-        /* XXX this is slow */
         if ((ret = yang_find_prefix_by_namespace(ymod, ns, &prefix)) < 0)
             goto done;
         if (ret == 0) /* not found (this may happen in augment and maybe should be treated otherwise) */
diff --git a/lib/src/clixon_yang_internal.h b/lib/src/clixon_yang_internal.h
index 3a2a59cc..2bbda263 100644
--- a/lib/src/clixon_yang_internal.h
+++ b/lib/src/clixon_yang_internal.h
@@ -113,4 +113,3 @@ struct yang_stmt{
 };
 
 #endif  /* _CLIXON_YANG_INTERNAL_H_ */
-