diff --git a/CHANGELOG.md b/CHANGELOG.md index 42aa51cc..824a25ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,11 @@ Users may have to change how they access the system * New clixon-config@2020-08-17.yang revision * Added `CLICON_RESTCONF_ADDRESS` for setting evhtp bind address +### Corrected Bugs + +* Fixed: [(CLI) the description of a used grouping is shown instead of the encapsulating container #124](https://github.com/clicon/clixon/issues/124) + * Uses/group and augments only copies *schemanodes*. This means reference/description/.. etc are not copied, the original is kept. Also, as a side-effect of the bugfix, a final cardinality sanity check is now made after all yang modifications, not only at the time the file is loaded. + ## 4.6.0 14 August 2020 diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c index 6765c383..15ab9767 100644 --- a/lib/src/clixon_yang.c +++ b/lib/src/clixon_yang.c @@ -362,19 +362,36 @@ ys_new(enum rfc_6020 keyw) return ys; } -/*! Free a single yang statement */ +/*! Free a single yang statement, dont remove children + * + * @param[in] ys Yang node to remove + * @param[in] self Free own node + * @retval 0 OK + * @retval -1 Error + * @see ys_free + */ static int -ys_free1(yang_stmt *ys) +ys_free1(yang_stmt *ys, + int self) { - if (ys->ys_argument) + if (ys->ys_argument){ free(ys->ys_argument); - if (ys->ys_cv) + ys->ys_argument = NULL; + } + if (ys->ys_cv){ cv_free(ys->ys_cv); - if (ys->ys_cvec) + ys->ys_cv = NULL; + } + if (ys->ys_cvec){ cvec_free(ys->ys_cvec); - if (ys->ys_typecache) + ys->ys_cvec = NULL; + } + if (ys->ys_typecache){ yang_type_cache_free(ys->ys_typecache); - free(ys); + ys->ys_typecache = NULL; + } + if (self) + free(ys); return 0; } @@ -422,7 +439,7 @@ ys_free(yang_stmt *ys) } if (ys->ys_stmt) free(ys->ys_stmt); - ys_free1(ys); + ys_free1(ys, 1); return 0; } @@ -461,11 +478,14 @@ yn_realloc(yang_stmt *yn) /*! Copy yang statement recursively from old to new * @param[in] ynew New empty (but created) yang statement (to) * @param[in] yold Old existing yang statement (from) + * @retval 0 OK + * @retval -1 Error * @code * yang_stmt *new = ys_new(Y_LEAF); * if (ys_cp(new, old) < 0) * err; * @endcode + * @see ys_replace */ int ys_cp(yang_stmt *ynew, @@ -519,6 +539,8 @@ ys_cp(yang_stmt *ynew, * @param[in] old Old existing yang statement (from) * @retval NULL Error * @retval nw New created yang statement + * @retval 0 OK + * @retval -1 Error * This may involve duplicating strings, etc. * The new yang tree needs to be freed by ys_free(). * The parent of new is NULL, it needs to be explicityl inserted somewhere @@ -541,6 +563,47 @@ ys_dup(yang_stmt *old) return nw; } +/*! Replace yold with ynew (insert ynew at the exact place of yold). Keep yold pointer as-is. + * + * @param[in] yorig Existing yang statement + * @param[in] yfrom New empty (but created) yang statement + + * @retval 0 OK + * @retval -1 Error + * @code + * if (ys_replace(new, old) < 0) + * err; + * @endcode + * @see ys_cp + * @note yfrom is left in its original state + */ +int +ys_replace(yang_stmt *yorig, + yang_stmt *yfrom) +{ + int retval = -1; + yang_stmt *yp; /* parent */ + yang_stmt *yc; /* child */ + + yp = yang_parent_get(yorig); + /* Remove old yangs all children */ + yc = NULL; + while ((yc = yn_each(yorig, yc)) != NULL) + ys_free(yc); + if (yorig->ys_stmt){ + free(yorig->ys_stmt); + yorig->ys_stmt = NULL; + yorig->ys_len = 0; + } + ys_free1(yorig, 0); /* Remove all in yold except the actual object */ + if (ys_cp(yorig, yfrom) < 0) + goto done; + yorig->ys_parent = yp; + retval = 0; + done: + return retval; +} + /*! Append yang statement as child of a parent yang_statement, last in list * * @param[in] ys_parent Add child to this parent diff --git a/lib/src/clixon_yang_parse_lib.c b/lib/src/clixon_yang_parse_lib.c index 37f39add..f91c2ab4 100644 --- a/lib/src/clixon_yang_parse_lib.c +++ b/lib/src/clixon_yang_parse_lib.c @@ -176,11 +176,11 @@ yang_augment_node(yang_stmt *ys, goto done; if (ytarget == NULL) goto ok; - /* Extend ytarget with ys' children - * First enlarge ytarget vector - */ + /* Extend ytarget with ys' schemanode children */ yc0 = NULL; while ((yc0 = yn_each(ys, yc0)) != NULL) { + if (!yang_schemanode(yc0)) + continue; if ((yc = ys_dup(yc0)) == NULL) goto done; yc->ys_mymodule = ymod; @@ -316,6 +316,7 @@ yang_expand_grouping(yang_stmt *yn) int glen; int i; int j; + int k; char *id = NULL; char *prefix = NULL; size_t size; @@ -346,8 +347,7 @@ yang_expand_grouping(yang_stmt *yn) goto done; break; } - /* Check so that this uses statement is not a decendant of the grouping - * Not that there may be other indirect recursions (I think?) + /* Check so that this uses statement is not a descendant of the grouping */ yp = yn; do { @@ -373,10 +373,15 @@ yang_expand_grouping(yang_stmt *yn) */ if ((ygrouping2 = ys_dup(ygrouping)) == NULL) goto done; - /* Replace ys with ygrouping,... - * First enlarge parent vector + /* Only replace data/schemanodes: + * Compute the number of such nodes, and extend the child vector with that below */ - glen = yang_len_get(ygrouping2); + glen = 0; + yg = NULL; + while ((yg = yn_each(ygrouping2, yg)) != NULL) { + if (yang_schemanode(yg)) + glen++; + } /* * yn is parent: the children of ygrouping replaces ys. * Is there a case when glen == 0? YES AND THIS BREAKS @@ -418,11 +423,18 @@ yang_expand_grouping(yang_stmt *yn) * grouping. I interpret that as only one node --> break */ break; } - /* Then copy and insert each child element */ - for (j=0; jys_stmt[j]; /* Child of refined copy */ - yn->ys_stmt[i+j] = yg; + /* Only replace data/schemanodes */ + if (!yang_schemanode(yg)){ + ys_free(yg); + continue; + } + yn->ys_stmt[i+k] = yg; yg->ys_parent = yn; + k++; } /* Remove 'uses' node */ ys_free(ys); @@ -1067,6 +1079,10 @@ yang_parse_post(clicon_handle h, if (ys_list_check(h, yspec->ys_stmt[i]) < 0) goto done; } + /* 9. Check cardinality a second time after grouping/augment etc */ + for (i=modnr; iys_stmt[i], yang_argument_get(yspec->ys_stmt[i])) < 0) + goto done; retval = 0; done: return retval; diff --git a/test/lib.sh b/test/lib.sh index c01dbcb8..1c2604f3 100755 --- a/test/lib.sh +++ b/test/lib.sh @@ -297,11 +297,16 @@ wait_restconf(){ fi } -# Increment test number and print a nice string -new(){ +endtest() +{ if [ $valgrindtest -eq 1 ]; then checkvalgrind fi +} + +# Increment test number and print a nice string +new(){ + endtest # finalize previous test testnr=`expr $testnr + 1` testi=`expr $testi + 1` testname=$1 diff --git a/test/test_feature.sh b/test/test_feature.sh index 7418715f..7314dfbd 100755 --- a/test/test_feature.sh +++ b/test/test_feature.sh @@ -303,4 +303,6 @@ if [ $BE -ne 0 ]; then stop_backend -f $cfg fi +endtest + rm -rf $dir diff --git a/test/test_identity.sh b/test/test_identity.sh index 918f0e5e..a59ef42c 100755 --- a/test/test_identity.sh +++ b/test/test_identity.sh @@ -321,16 +321,16 @@ if [ $RC -ne 0 ]; then fi if [ $BE -eq 0 ]; then - exit # BE + new "Kill backend" + # Check if premature kill + pid=$(pgrep -u root -f clixon_backend) + if [ -z "$pid" ]; then + err "backend already dead" + fi + # kill backend + stop_backend -f $cfg fi -new "Kill backend" -# Check if premature kill -pid=$(pgrep -u root -f clixon_backend) -if [ -z "$pid" ]; then - err "backend already dead" -fi -# kill backend -stop_backend -f $cfg +endtest rm -rf $dir diff --git a/test/test_insert.sh b/test/test_insert.sh index e5fa3570..20095b3a 100755 --- a/test/test_insert.sh +++ b/test/test_insert.sh @@ -214,7 +214,10 @@ testrun "$x0" "32" new "adv list add leaf-list" testrun "$x0" "32" +endtest + rm -rf $dir # unset conditional parameters unset clixon_util_xml_mod + diff --git a/test/test_nacm_default.sh b/test/test_nacm_default.sh index 5c990b64..650df4f5 100755 --- a/test/test_nacm_default.sh +++ b/test/test_nacm_default.sh @@ -225,6 +225,8 @@ for db in startup init; do done +endtest + rm -rf $dir # unset conditional parameters diff --git a/test/test_nacm_recovery.sh b/test/test_nacm_recovery.sh index e70e72bf..fa4fd94e 100755 --- a/test/test_nacm_recovery.sh +++ b/test/test_nacm_recovery.sh @@ -223,4 +223,6 @@ RECOVERY=_recovery new "cred: $CRED realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" testrun $CRED $REALUSER $PSEUDO $RECOVERY false false +endtest + rm -rf $dir