From 2f88ef3ed69b09ce87ef515d59f6053df74684de Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Fri, 3 Nov 2023 14:06:41 +0100 Subject: [PATCH] Fixed: [cl:creator attribute must be persistent](https://github.com/clicon/clixon-controller/issues/54) --- CHANGELOG.md | 1 + lib/clixon/clixon_xml.h | 3 +- lib/src/clixon_datastore_write.c | 8 ++-- lib/src/clixon_xml.c | 68 ++++++++++++++++++++++++++++++-- lib/src/clixon_xml_map.c | 55 ++++++++++++-------------- 5 files changed, 97 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cd6a784..01b923ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -76,6 +76,7 @@ Developers may need to change their code ### Corrected Bugs +* Fixed: [cl:creator attribute must be persistent](https://github.com/clicon/clixon-controller/issues/54) * Fixed: [Does clixon cli support autocompletion for leafrefs pointed to another module?](https://github.com/clicon/clixon/issues/455) * Fixed: [commit diff sometimes includes namespace in output](https://github.com/clicon/clixon-controller/issues/44) diff --git a/lib/clixon/clixon_xml.h b/lib/clixon/clixon_xml.h index 53f2b007..9ef7201d 100644 --- a/lib/clixon/clixon_xml.h +++ b/lib/clixon/clixon_xml.h @@ -234,7 +234,8 @@ int xml_creator_add(cxobj *xn, char *name); int xml_creator_rm(cxobj *xn, char *name); int xml_creator_find(cxobj *xn, char *name); size_t xml_creator_len(cxobj *xn); -int xml_creator_copy(cxobj *x0, cxobj *x1); +int xml_creator_copy_one(cxobj *x0, cxobj *x1); +int xml_creator_copy_all(cxobj *x0, cxobj *x1); int xml_creator_print(FILE *f, cxobj *xn); char *xml_value(cxobj *xn); diff --git a/lib/src/clixon_datastore_write.c b/lib/src/clixon_datastore_write.c index 88c1a5a3..ba590a50 100644 --- a/lib/src/clixon_datastore_write.c +++ b/lib/src/clixon_datastore_write.c @@ -798,8 +798,12 @@ text_modify(clicon_handle h, } /* XXX: Note, if there is an error in adding the object later, the * original object is not reverted. + * XXX: Here creator attributes in x0 are destroyed */ if (x0){ + /* Recursively copy creator attributes from existing tree */ + if (xml_creator_copy_all(x0, x1) < 0) + goto done; xml_purge(x0); x0 = NULL; } @@ -849,7 +853,7 @@ text_modify(clicon_handle h, #ifdef XML_PARENT_CANDIDATE xml_parent_candidate_set(x0, x0p); #endif - if (xml_creator_copy(x1, x0) < 0) + if (xml_creator_copy_one(x1, x0) < 0) goto done; changed++; /* Get namespace from x1 @@ -1257,12 +1261,10 @@ xmldb_put(clicon_handle h, goto done; } /* Here x0 looks like: ... */ - #if 0 /* debug */ if (xml_apply0(x1, -1, xml_sort_verify, NULL) < 0) clicon_log(LOG_NOTICE, "%s: verify failed #1", __FUNCTION__); #endif - xnacm = clicon_nacm_cache(h); permit = (xnacm==NULL); diff --git a/lib/src/clixon_xml.c b/lib/src/clixon_xml.c index c237c43b..11a2f6d5 100644 --- a/lib/src/clixon_xml.c +++ b/lib/src/clixon_xml.c @@ -723,7 +723,7 @@ xml_creator_len(cxobj *xn) return 0; } -/*! Copy creator info from x0 to x1 +/*! Copy creator info from x0 to x1 single object * * @param[in] x0 Source XML node * @param[in] x1 Destination XML node @@ -731,8 +731,8 @@ xml_creator_len(cxobj *xn) * @retval -1 Error */ int -xml_creator_copy(cxobj *x0, - cxobj *x1) +xml_creator_copy_one(cxobj *x0, + cxobj *x1) { int retval = -1; @@ -746,6 +746,66 @@ xml_creator_copy(cxobj *x0, return retval; } +/*! Recursively copy creator attributes from existing tree based on equivalence algorithm + * + * @param[in] x0 Source XML node + * @param[in] x1 Destination XML node + * @retval 0 OK + * @retval -1 Error + * @see xml_tree_equal equivalence algorithm + */ +int +xml_creator_copy_all(cxobj *x0, + cxobj *x1) +{ + int retval = -1; + int eq; + cxobj *x0c; /* x0 child */ + cxobj *x1c; /* x1 child */ + yang_stmt *yc0; + yang_stmt *yc1; + + /* Traverse x0 and x1 in lock-step */ + x0c = x1c = NULL; + x0c = xml_child_each(x0, x0c, CX_ELMNT); + x1c = xml_child_each(x1, x1c, CX_ELMNT); + for (;;){ + if (x0c == NULL || x1c == NULL) + goto ok; + eq = xml_cmp(x0c, x1c, 0, 0, NULL); + if (eq < 0){ + x0c = xml_child_each(x0, x0c, CX_ELMNT); + continue; + } + else if (eq > 0){ + x1c = xml_child_each(x1, x1c, CX_ELMNT); + continue; + } + else { /* equal */ + /* Both x0c and x1c exists and are equal, check if they are yang-equal. */ + yc0 = xml_spec(x0c); + yc1 = xml_spec(x1c); + if (yc0 && yc1 && yc0 != yc1){ /* choice */ + ; + } + else { + if (x0c->x_creators){ + if (xml_creator_copy_one(x0c, x1c) < 0) + goto done; + } + else if (xml_creator_copy_all(x0c, x1c) < 0) + goto done; + } + } + x0c = xml_child_each(x0, x0c, CX_ELMNT); + x1c = xml_child_each(x1, x1c, CX_ELMNT); + } + ok: + retval = 0; + done: + return retval; +} + /*! Print XML and creator tags where they exists, apply help function * * @param[in] x XML tree @@ -2154,7 +2214,7 @@ xml_copy_one(cxobj *x0, switch (xml_type(x0)){ case CX_ELMNT: xml_spec_set(x1, xml_spec(x0)); - if (xml_creator_copy(x0, x1) < 0) + if (xml_creator_copy_one(x0, x1) < 0) goto done; break; case CX_BODY: diff --git a/lib/src/clixon_xml_map.c b/lib/src/clixon_xml_map.c index 9904e988..dd0bdcf1 100644 --- a/lib/src/clixon_xml_map.c +++ b/lib/src/clixon_xml_map.c @@ -368,28 +368,27 @@ xml_diff1(cxobj *x0, if (cxvec_append(x1c, x1vec, x1veclen) < 0) goto done; } - else - if (yc0 && yang_keyword_get(yc0) == Y_LEAF){ - /* if x0c and x1c are leafs w bodies, then they may be changed */ - b0 = xml_body(x0c); - b1 = xml_body(x1c); - if (b0 == NULL && b1 == NULL) - ; - else if (b0 == NULL || b1 == NULL - || strcmp(b0, b1) != 0 - ){ - if (cxvec_append(x0c, changed_x0, changedlen) < 0) - goto done; - (*changedlen)--; /* append two vectors */ - if (cxvec_append(x1c, changed_x1, changedlen) < 0) - goto done; - } + else if (yc0 && yang_keyword_get(yc0) == Y_LEAF){ + /* if x0c and x1c are leafs w bodies, then they may be changed */ + b0 = xml_body(x0c); + b1 = xml_body(x1c); + if (b0 == NULL && b1 == NULL) + ; + else if (b0 == NULL || b1 == NULL + || strcmp(b0, b1) != 0 + ){ + if (cxvec_append(x0c, changed_x0, changedlen) < 0) + goto done; + (*changedlen)--; /* append two vectors */ + if (cxvec_append(x1c, changed_x1, changedlen) < 0) + goto done; } - else if (xml_diff1(x0c, x1c, - x0vec, x0veclen, - x1vec, x1veclen, - changed_x0, changed_x1, changedlen)< 0) - goto done; + } + else if (xml_diff1(x0c, x1c, + x0vec, x0veclen, + x1vec, x1veclen, + changed_x0, changed_x1, changedlen)< 0) + goto done; } x0c = xml_child_each(x0, x0c, CX_ELMNT); x1c = xml_child_each(x1, x1c, CX_ELMNT); @@ -474,8 +473,8 @@ xml_tree_equal(cxobj *x0, yang_stmt *yc1; char *b0; char *b1; - cxobj *x0c = NULL; /* x0 child */ - cxobj *x1c = NULL; /* x1 child */ + cxobj *x0c; /* x0 child */ + cxobj *x1c; /* x1 child */ int extflag = 0; /* Traverse x0 and x1 in lock-step */ @@ -486,7 +485,7 @@ xml_tree_equal(cxobj *x0, if (x0c == NULL && x1c == NULL) goto ok; else if (x0c == NULL){ - /* If cl:gnore-compare extension, return equal */ + /* If cl:ignore-compare extension, return equal */ if ((yc1 = xml_spec(x1c)) != NULL){ if (yang_extension_value(yc1, "ignore-compare", CLIXON_LIB_NS, &extflag, NULL) < 0) goto done; @@ -511,16 +510,12 @@ xml_tree_equal(cxobj *x0, goto ok; } /* Both x0c and x1c exists, check if they are yang-equal. */ - eq = xml_cmp(x0c, x1c, 0, 0, NULL); - if (eq < 0){ - goto done; - } - else if (eq > 0){ + if ((eq = xml_cmp(x0c, x1c, 0, 0, NULL)) != 0){ goto done; } else{ /* equal */ /* xml-spec NULL could happen with anydata children for example, - * if so, continute compare children but without yang + * if so, continue compare children but without yang */ yc0 = xml_spec(x0c); yc1 = xml_spec(x1c);