validation OPTIMIZE avoid finding HT record twice

Required changes to lyd_find_with_col_cb function
behavior.

Author: michalvasko

src/hash_table.c

@@ -309,14 +309,14 @@ lyht_find_with_val_cb(const struct ly_ht *ht, void *val_p, uint32_t hash, lyht_v
 
 LIBYANG_API_DEF LY_ERR
 lyht_find_next_with_collision_cb(const struct ly_ht *ht, void *val_p, uint32_t hash,
-        lyht_value_equal_cb collision_val_equal, void **match_p)
+        lyht_value_equal_cb val_equal, void **match_p)
 {
     struct ly_ht_rec *rec;
     uint32_t rec_idx;
     uint32_t i;
 
     /* find the record of the previously found value */
-    if (lyht_find_rec(ht, val_p, hash, 1, ht->val_equal, &i, &rec)) {
+    if (lyht_find_rec(ht, val_p, hash, 1, val_equal ? val_equal : ht->val_equal, &i, &rec)) {
         /* not found, cannot happen */
         LOGINT_RET(NULL);
     }
@@ -329,8 +329,8 @@ lyht_find_next_with_collision_cb(const struct ly_ht *ht, void *val_p, uint32_t h
             continue;
         }
 
-        if (collision_val_equal) {
-            if (collision_val_equal(val_p, &rec->val, 0, ht->cb_data)) {
+        if (val_equal) {
+            if (val_equal(val_p, &rec->val, 0, ht->cb_data)) {
                 /* even the value matches */
                 if (match_p) {
                     *match_p = rec->val;

src/hash_table.h

@@ -163,13 +163,14 @@ LIBYANG_API_DECL LY_ERR lyht_find_next(const struct ly_ht *ht, void *val_p, uint
  * @param[in] ht Hash table to search in.
  * @param[in] val_p Pointer to the previously found value in @p ht.
  * @param[in] hash Hash of the previously found value.
- * @param[in] collision_val_equal Val equal callback to use for checking collisions.
+ * @param[in] val_equal Callback for checking value equivalence. Called with @p mod 1 when searching for the first value
+ * and then uses @p mod 0 to check all the following collisions.
  * @param[out] match_p Pointer to the matching value, optional.
  * @return LY_SUCCESS if value was found,
  * @return LY_ENOTFOUND if not found.
  */
 LIBYANG_API_DECL LY_ERR lyht_find_next_with_collision_cb(const struct ly_ht *ht, void *val_p, uint32_t hash,
-        lyht_value_equal_cb collision_val_equal, void **match_p);
+        lyht_value_equal_cb val_equal, void **match_p);
 
 /**
  * @brief Insert a value into a hash table.

src/printer_lyb.c

@@ -70,6 +70,18 @@ lyb_ptr_equal_cb(void *val1_p, void *val2_p, ly_bool UNUSED(mod), void *UNUSED(c
     return 0;
 }
 
+/**
+ * @brief Hash table equal callback for checking collisions.
+ *
+ * Implementation of ::lyht_value_equal_cb.
+ */
+static ly_bool
+lyb_col_equal_cb(void *val1_p, void *val2_p, ly_bool mod, void *cb_data)
+{
+    /* for first value check use lyb_ptr_equal_cb, for collisions lyb_hash_equal_cb */
+    return mod ? lyb_ptr_equal_cb(val1_p, val2_p, mod, cb_data) : lyb_hash_equal_cb(val1_p, val2_p, mod, cb_data);
+}
+
 /**
  * @brief Check that sibling collision hash is safe to insert into hash table.
  *
@@ -91,7 +103,6 @@ lyb_hash_sequence_check(struct ly_ht *ht, struct lysc_node *sibling, LYB_HASH ht
         return LY_SUCCESS;
     }
 
-    lyht_set_cb(ht, lyb_ptr_equal_cb);
     do {
         int64_t j;
 
@@ -103,15 +114,13 @@ lyb_hash_sequence_check(struct ly_ht *ht, struct lysc_node *sibling, LYB_HASH ht
         }
         if (j == -1) {
             /* all whole hash sequences of nodes inserted with last hash col ID compare_col_id collide */
-            lyht_set_cb(ht, lyb_hash_equal_cb);
             return LY_EEXIST;
         }
 
         /* get next node inserted with last hash col ID ht_col_id */
-    } while (!lyht_find_next_with_collision_cb(ht, col_node, lyb_get_hash(*col_node, ht_col_id), lyb_hash_equal_cb,
+    } while (!lyht_find_next_with_collision_cb(ht, col_node, lyb_get_hash(*col_node, ht_col_id), lyb_col_equal_cb,
             (void **)&col_node));
 
-    lyht_set_cb(ht, lyb_hash_equal_cb);
     return LY_SUCCESS;
 }
 

src/validation.c

@@ -533,6 +533,18 @@ lyd_validate_unres(struct lyd_node **tree, const struct lys_module *mod, enum ly
     return rc;
 }
 
+/**
+ * @brief Compare callback for finding duplicates in hash table.
+ *
+ * Implementation of ::lyht_value_equal_cb.
+ */
+static ly_bool
+lyd_val_dup_val_equal(void *val1_p, void *val2_p, ly_bool UNUSED(mod), void *cb_data)
+{
+    /* always find the same instance, not the exact same pointer (set mod = 0) */
+    return lyd_hash_table_val_equal(val1_p, val2_p, 0, cb_data);
+}
+
 /**
  * @brief Validate instance duplication.
  *
@@ -544,7 +556,6 @@ lyd_validate_unres(struct lyd_node **tree, const struct lys_module *mod, enum ly
 static LY_ERR
 lyd_validate_duplicates(const struct lyd_node *first, const struct lyd_node *node, uint32_t val_opts)
 {
-    struct lyd_node **match_p, *match;
     ly_bool fail = 0;
 
     assert(node->flags & LYD_NEW);
@@ -557,12 +568,9 @@ lyd_validate_duplicates(const struct lyd_node *first, const struct lyd_node *nod
 
     /* find exactly the same next instance using hashes if possible */
     if (node->parent && node->parent->children_ht) {
-        lyd_find_sibling_first(first, node, &match);
-        assert(match);
-
-        if (match != node) {
-            fail = 1;
-        } else if (!lyht_find_next(node->parent->children_ht, &node, node->hash, (void **)&match_p)) {
+        /* because of the callback used, an instance must always be found (pointer may or may not be equal to node),
+         * so if we find another instance, there is a duplicate */
+        if (!lyht_find_next_with_collision_cb(node->parent->children_ht, &node, node->hash, lyd_val_dup_val_equal, NULL)) {
             fail = 1;
         }
     } else {