Skip to content

Commit

Permalink
dict BUGFIX revert ffeedfd
Browse files Browse the repository at this point in the history
This optimization exposes another bug that it is not
possible to solve properly without NBC changes so
wait with it until a new SO version.
  • Loading branch information
michalvasko committed Aug 16, 2024
1 parent ffeedfd commit 5939112
Show file tree
Hide file tree
Showing 14 changed files with 53 additions and 100 deletions.
55 changes: 7 additions & 48 deletions src/dict.c
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ lydict_resize_val_eq(void *val1_p, void *val2_p, ly_bool mod, void *UNUSED(cb_da
str1 = ((struct ly_dict_rec *)val1_p)->value;
str2 = ((struct ly_dict_rec *)val2_p)->value;

LY_CHECK_ERR_RET(!str1, LOGARG(NULL, val1_p), 0);
LY_CHECK_ERR_RET(!str2, LOGARG(NULL, val2_p), 0);

if (mod) {
/* used when inserting new values */
if (strcmp(str1, str2) == 0) {
Expand Down Expand Up @@ -174,7 +177,7 @@ lydict_remove(const struct ly_ctx *ctx, const char *value)
return ret;
}

static LY_ERR
LY_ERR
dict_insert(const struct ly_ctx *ctx, char *value, size_t len, ly_bool zerocopy, const char **str_p)
{
LY_ERR ret = LY_SUCCESS;
Expand Down Expand Up @@ -218,7 +221,9 @@ dict_insert(const struct ly_ctx *ctx, char *value, size_t len, ly_bool zerocopy,
return ret;
}

*str_p = match->value;
if (str_p) {
*str_p = match->value;
}

return ret;
}
Expand Down Expand Up @@ -264,49 +269,3 @@ lydict_insert_zc(const struct ly_ctx *ctx, char *value, const char **str_p)

return result;
}

static LY_ERR
dict_dup(const struct ly_ctx *ctx, char *value, const char **str_p)
{
LY_ERR ret = LY_SUCCESS;
struct ly_dict_rec *match = NULL, rec;
uint32_t hash;

/* set new callback to only compare memory addresses */
lyht_value_equal_cb prev = lyht_set_cb(ctx->dict.hash_tab, lydict_resize_val_eq);

LOGDBG(LY_LDGDICT, "duplicating %s", value);
hash = lyht_hash(value, strlen(value));
rec.value = value;

ret = lyht_find(ctx->dict.hash_tab, (void *)&rec, hash, (void **)&match);
if (ret == LY_SUCCESS) {
/* record found, increase refcount */
match->refcount++;
*str_p = match->value;
}

/* restore callback */
lyht_set_cb(ctx->dict.hash_tab, prev);

return ret;
}

LIBYANG_API_DEF LY_ERR
lydict_dup(const struct ly_ctx *ctx, const char *value, const char **str_p)
{
LY_ERR result;

LY_CHECK_ARG_RET(ctx, ctx, str_p, LY_EINVAL);

if (!value) {
*str_p = NULL;
return LY_SUCCESS;
}

pthread_mutex_lock((pthread_mutex_t *)&ctx->dict.lock);
result = dict_dup(ctx, (char *)value, str_p);
pthread_mutex_unlock((pthread_mutex_t *)&ctx->dict.lock);

return result;
}
12 changes: 0 additions & 12 deletions src/dict.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,18 +113,6 @@ LIBYANG_API_DECL LY_ERR lydict_insert_zc(const struct ly_ctx *ctx, char *value,
*/
LIBYANG_API_DECL LY_ERR lydict_remove(const struct ly_ctx *ctx, const char *value);

/**
* @brief Duplicate string in dictionary. Only a reference counter is incremented.
*
* @param[in] ctx libyang context handler
* @param[in] value NULL-terminated string to be duplicated in the dictionary (reference counter is incremented).
* @param[out] str_p Optional parameter to get pointer to the string corresponding to the @p value and stored in dictionary.
* @return LY_SUCCESS in case the string already exists in the dictionary.
* @return LY_ENOTFOUND in case the string was not found.
* @return LY_ERR on other errors
*/
LIBYANG_API_DECL LY_ERR lydict_dup(const struct ly_ctx *ctx, const char *value, const char **str_p);

/** @} dict */

#ifdef __cplusplus
Expand Down
26 changes: 14 additions & 12 deletions src/path.c
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,7 @@ ly_path_compile_predicate(const struct ly_ctx *ctx, const struct lysc_node *cur_

/* store the value */
LOG_LOCSET(key, NULL);
ret = lyd_value_store(ctx_node->module->ctx, &p->value, ((struct lysc_node_leaf *)key)->type, val, val_len, 0, 0,
ret = lyd_value_store(ctx, &p->value, ((struct lysc_node_leaf *)key)->type, val, val_len, 0, 0,
NULL, format, prefix_data, LYD_HINT_DATA, key, NULL);
LOG_LOCBACK(1, 0);
LY_CHECK_ERR_GOTO(ret, p->value.realtype = NULL, cleanup);
Expand Down Expand Up @@ -736,7 +736,7 @@ ly_path_compile_predicate(const struct ly_ctx *ctx, const struct lysc_node *cur_
/* names (keys) are unique - it was checked when parsing */
LOGVAL(ctx, LYVE_XPATH, "Predicate missing for a key of %s \"%s\" in path.",
lys_nodetype2str(ctx_node->nodetype), ctx_node->name);
ly_path_predicates_free(ctx_node->module->ctx, *predicates);
ly_path_predicates_free(ctx, *predicates);
*predicates = NULL;
ret = LY_EVALID;
goto cleanup;
Expand Down Expand Up @@ -771,10 +771,12 @@ ly_path_compile_predicate(const struct ly_ctx *ctx, const struct lysc_node *cur_
}

/* store the value */
LOG_LOCSET(ctx_node, NULL);
ret = lyd_value_store(ctx_node->module->ctx, &p->value, ((struct lysc_node_leaflist *)ctx_node)->type, val, val_len, 0, 0,
if (ctx_node) {
LOG_LOCSET(ctx_node, NULL);
}
ret = lyd_value_store(ctx, &p->value, ((struct lysc_node_leaflist *)ctx_node)->type, val, val_len, 0, 0,
NULL, format, prefix_data, LYD_HINT_DATA, ctx_node, NULL);
LOG_LOCBACK(1, 0);
LOG_LOCBACK(ctx_node ? 1 : 0, 0);
LY_CHECK_ERR_GOTO(ret, p->value.realtype = NULL, cleanup);
++(*tok_idx);

Expand Down Expand Up @@ -1091,15 +1093,15 @@ ly_path_compile_deref(const struct ly_ctx *ctx, const struct lysc_node *ctx_node
}
lref = (const struct lysc_type_leafref *)deref_leaf_node->type;
LY_CHECK_GOTO(ret = ly_path_append(ctx, path2, path), cleanup);
ly_path_free(path2);
ly_path_free(ctx, path2);
path2 = NULL;

/* compile dereferenced leafref expression and append it to the path */
LY_CHECK_GOTO(ret = ly_path_compile_leafref(ctx, node2, top_ext, lref->path, oper, target, format, prefix_data,
&path2), cleanup);
node2 = path2[LY_ARRAY_COUNT(path2) - 1].node;
LY_CHECK_GOTO(ret = ly_path_append(ctx, path2, path), cleanup);
ly_path_free(path2);
ly_path_free(ctx, path2);
path2 = NULL;

/* properly parsed path must always continue with ')' and '/' */
Expand All @@ -1123,9 +1125,9 @@ ly_path_compile_deref(const struct ly_ctx *ctx, const struct lysc_node *ctx_node
LY_CHECK_GOTO(ret = ly_path_append(ctx, path2, path), cleanup);

cleanup:
ly_path_free(path2);
ly_path_free(ctx, path2);
if (ret) {
ly_path_free(*path);
ly_path_free(ctx, *path);
*path = NULL;
}
return ret;
Expand Down Expand Up @@ -1281,7 +1283,7 @@ _ly_path_compile(const struct ly_ctx *ctx, const struct lys_module *cur_mod, con

cleanup:
if (ret) {
ly_path_free(*path);
ly_path_free(ctx, *path);
*path = NULL;
}
LOG_LOCBACK(cur_node ? 1 : 0, 0);
Expand Down Expand Up @@ -1486,7 +1488,7 @@ ly_path_predicates_free(const struct ly_ctx *ctx, struct ly_path_predicate *pred
}

void
ly_path_free(struct ly_path *path)
ly_path_free(const struct ly_ctx *ctx, struct ly_path *path)
{
LY_ARRAY_COUNT_TYPE u;

Expand All @@ -1495,7 +1497,7 @@ ly_path_free(struct ly_path *path)
}

LY_ARRAY_FOR(path, u) {
ly_path_predicates_free(path[u].node->module->ctx, path[u].predicates);
ly_path_predicates_free(ctx, path[u].predicates);
}
LY_ARRAY_FREE(path);
}
3 changes: 2 additions & 1 deletion src/path.h
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,9 @@ void ly_path_predicates_free(const struct ly_ctx *ctx, struct ly_path_predicate
/**
* @brief Free ly_path structure.
*
* @param[in] ctx libyang context.
* @param[in] path The structure ([sized array](@ref sizedarrays)) to free.
*/
void ly_path_free(struct ly_path *path);
void ly_path_free(const struct ly_ctx *ctx, struct ly_path *path);

#endif /* LY_PATH_H_ */
18 changes: 6 additions & 12 deletions src/plugins_types.c
Original file line number Diff line number Diff line change
Expand Up @@ -352,16 +352,10 @@ lyplg_type_print_simple(const struct ly_ctx *UNUSED(ctx), const struct lyd_value
LIBYANG_API_DEF LY_ERR
lyplg_type_dup_simple(const struct ly_ctx *ctx, const struct lyd_value *original, struct lyd_value *dup)
{
LY_ERR r;

if ((r = lydict_dup(ctx, original->_canonical, &dup->_canonical))) {
/* in case of error NULL the values so that freeing does not fail */
memset(dup, 0, sizeof *dup);
return r;
}
memset(dup, 0, sizeof *dup);
LY_CHECK_RET(lydict_insert(ctx, original->_canonical, 0, &dup->_canonical));
memcpy(dup->fixed_mem, original->fixed_mem, sizeof dup->fixed_mem);
dup->realtype = original->realtype;

return LY_SUCCESS;
}

Expand Down Expand Up @@ -894,17 +888,17 @@ lyplg_type_lypath_new(const struct ly_ctx *ctx, const char *value, size_t value_
ly_err_clean((struct ly_ctx *)ctx, e);
}

ly_path_free(*path);
ly_path_free(ctx, *path);
*path = NULL;
}

return ret;
}

LIBYANG_API_DEF void
lyplg_type_lypath_free(const struct ly_ctx *UNUSED(ctx), struct ly_path *path)
lyplg_type_lypath_free(const struct ly_ctx *ctx, struct ly_path *path)
{
ly_path_free(path);
ly_path_free(ctx, path);
}

LIBYANG_API_DEF LY_ERR
Expand Down Expand Up @@ -1013,7 +1007,7 @@ lyplg_type_resolve_leafref_get_target_path(const struct lyxp_expr *path, const s
LY_CHECK_GOTO(lyxp_expr_parse(ctx_node->module->ctx, str_path, 0, 1, target_path), cleanup);

cleanup:
ly_path_free(p);
ly_path_free(ctx_node->module->ctx, p);
free(str_path);
return rc;
}
Expand Down
2 changes: 1 addition & 1 deletion src/plugins_types/instanceid.c
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ lyplg_type_free_instanceid(const struct ly_ctx *ctx, struct lyd_value *value)
{
lydict_remove(ctx, value->_canonical);
value->_canonical = NULL;
ly_path_free(value->target);
ly_path_free(ctx, value->target);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/schema_compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -866,7 +866,7 @@ lys_compile_unres_leafref(struct lysc_ctx *ctx, const struct lysc_node *node, st

/* get the target node */
target = p[LY_ARRAY_COUNT(p) - 1].node;
ly_path_free(p);
ly_path_free(node->module->ctx, p);

if (!(target->nodetype & (LYS_LEAF | LYS_LEAFLIST))) {
LOGVAL(ctx->ctx, LYVE_REFERENCE, "Invalid leafref path \"%s\" - target node is %s instead of leaf or leaf-list.",
Expand Down Expand Up @@ -1456,7 +1456,7 @@ lys_compile_unres_depset(struct ly_ctx *ctx, struct lys_glob_unres *unres)
ret = ly_path_compile_leafref(cctx.ctx, l->node, cctx.ext, lref->path,
(l->node->flags & LYS_IS_OUTPUT) ? LY_PATH_OPER_OUTPUT : LY_PATH_OPER_INPUT, LY_PATH_TARGET_MANY,
LY_VALUE_SCHEMA_RESOLVED, lref->prefixes, &path);
ly_path_free(path);
ly_path_free(l->node->module->ctx, path);

assert(ret != LY_ERECOMPILE);
if (ret) {
Expand Down
2 changes: 1 addition & 1 deletion src/tree_data.c
Original file line number Diff line number Diff line change
Expand Up @@ -3624,7 +3624,7 @@ lyd_find_path(const struct lyd_node *ctx_node, const char *path, ly_bool output,

cleanup:
lyxp_expr_free(LYD_CTX(ctx_node), expr);
ly_path_free(lypath);
ly_path_free(LYD_CTX(ctx_node), lypath);
return ret;
}

Expand Down
2 changes: 1 addition & 1 deletion src/tree_data_new.c
Original file line number Diff line number Diff line change
Expand Up @@ -1811,7 +1811,7 @@ lyd_new_path_(struct lyd_node *parent, const struct ly_ctx *ctx, const struct ly
LY_ARRAY_INCREMENT(p);
}
}
ly_path_free(p);
ly_path_free(ctx, p);
if (!ret) {
/* set out params only on success */
if (new_parent) {
Expand Down
4 changes: 2 additions & 2 deletions src/tree_schema.c
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ lys_find_path_atoms(const struct ly_ctx *ctx, const struct lysc_node *ctx_node,
ret = lys_find_lypath_atoms(p, set);

cleanup:
ly_path_free(p);
ly_path_free(ctx, p);
lyxp_expr_free(ctx, expr);
return ret;
}
Expand Down Expand Up @@ -679,7 +679,7 @@ lys_find_path(const struct ly_ctx *ctx, const struct lysc_node *ctx_node, const
snode = p[LY_ARRAY_COUNT(p) - 1].node;

cleanup:
ly_path_free(p);
ly_path_free(ctx, p);
lyxp_expr_free(ctx, expr);
return snode;
}
Expand Down
2 changes: 1 addition & 1 deletion src/tree_schema_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -1829,7 +1829,7 @@ lysc_node_lref_target(const struct lysc_node *node)

/* get the target node */
target = p[LY_ARRAY_COUNT(p) - 1].node;
ly_path_free(p);
ly_path_free(node->module->ctx, p);

return target;
}
Expand Down
6 changes: 2 additions & 4 deletions src/xpath.c
Original file line number Diff line number Diff line change
Expand Up @@ -4042,7 +4042,7 @@ xpath_deref(struct lyxp_set **args, uint32_t UNUSED(arg_count), struct lyxp_set
if (!r) {
/* get the target node */
target = p[LY_ARRAY_COUNT(p) - 1].node;
ly_path_free(p);
ly_path_free(set->ctx, p);

LY_CHECK_RET(lyxp_set_scnode_insert_node(set, target, LYXP_NODE_ELEM, LYXP_AXIS_SELF, NULL));
} /* else the target was found before but is disabled so it was removed */
Expand Down Expand Up @@ -8272,9 +8272,7 @@ eval_name_test_with_predicate(const struct lyxp_expr *exp, uint32_t *tok_idx, en
options &= ~LYXP_SKIP_EXPR;
}
lydict_remove(set->ctx, ncname_dict);
if (predicates) {
ly_path_predicates_free(scnode->module->ctx, predicates);
}
ly_path_predicates_free(set->ctx, predicates);
return rc;
}

Expand Down
2 changes: 1 addition & 1 deletion tests/utests/data/test_tree_data.c
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ test_target(void **state)
assert_string_equal(lyd_get_value(term->prev), "b");

lyd_free_all(tree);
ly_path_free(path);
ly_path_free(UTEST_LYCTX, path);
lyxp_expr_free(UTEST_LYCTX, exp);
}

Expand Down
15 changes: 13 additions & 2 deletions tests/utests/schema/test_tree_schema_compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -1233,6 +1233,7 @@ test_type_instanceid(void **state)
{
struct lys_module *mod;
struct lysc_type *type;
//char *str;

assert_int_equal(LY_SUCCESS, lys_parse_mem(UTEST_LYCTX, "module a {namespace urn:a;prefix a;typedef mytype {type instance-identifier {require-instance false;}}"
"leaf l1 {type instance-identifier {require-instance true;}}"
Expand All @@ -1252,12 +1253,22 @@ test_type_instanceid(void **state)
assert_int_equal(LY_TYPE_INST, type->basetype);
assert_int_equal(1, ((struct lysc_type_instanceid *)type)->require_instance);

/* default value */
/* TODO needs a new SO for a proper fix; str = "module b1 {namespace urn:b1;prefix b1;"
"leaf l1 {type string;}}";
ly_ctx_set_module_imp_clb(UTEST_LYCTX, test_imp_clb, str);
ly_ctx_set_options(UTEST_LYCTX, LY_CTX_REF_IMPLEMENTED);
assert_int_equal(LY_SUCCESS, lys_parse_mem(UTEST_LYCTX, "module b2 {namespace urn:b2;prefix b2;"
"import b1 {prefix b1;}"
"leaf l1 {type instance-identifier; default \"/b1:l1\";}}", LYS_IN_YANG, NULL));
ly_ctx_set_options(UTEST_LYCTX, 0);*/

/* invalid cases */
assert_int_equal(LY_EVALID, lys_parse_mem(UTEST_LYCTX, "module aa {namespace urn:aa;prefix aa; leaf l {type instance-identifier {require-instance yes;}}}", LYS_IN_YANG, &mod));
assert_int_equal(LY_EVALID, lys_parse_mem(UTEST_LYCTX, "module aa {namespace urn:aa;prefix aa; leaf l {type instance-identifier {require-instance yes;}}}", LYS_IN_YANG, NULL));
CHECK_LOG_CTX("Parsing module \"aa\" failed.", NULL, 0);
CHECK_LOG_CTX("Invalid value \"yes\" of \"require-instance\".", NULL, 1);

assert_int_equal(LY_EVALID, lys_parse_mem(UTEST_LYCTX, "module aa {namespace urn:aa;prefix aa; leaf l {type instance-identifier {fraction-digits 1;}}}", LYS_IN_YANG, &mod));
assert_int_equal(LY_EVALID, lys_parse_mem(UTEST_LYCTX, "module aa {namespace urn:aa;prefix aa; leaf l {type instance-identifier {fraction-digits 1;}}}", LYS_IN_YANG, NULL));
CHECK_LOG_CTX("Invalid type restrictions for instance-identifier type.", "/aa:l", 0);
}

Expand Down

0 comments on commit 5939112

Please sign in to comment.