Fixed: [(CLI) the description of a used grouping is shown instead of the encapsulating container #124](https://github.com/clicon/clixon/issues/124)

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
 

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

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
+    /* Extend ytarget with ys' schemanode children */
-     * First enlarge ytarget vector 
-     */
     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
+	    /* Check so that this uses statement is not a descendant of the grouping
-	     * Not that there may be other indirect recursions (I think?)
 	     */
 	    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,... 
+	    /* Only replace data/schemanodes:
-	     * First enlarge parent vector 
+	     * 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 */
+	    /* Then copy and insert each child element from ygrouping2 to yn */
-	    for (j=0; j<glen; j++){
+	    k=0;
+	    for (j=0; j<yang_len_get(ygrouping2); j++){
 		yg = ygrouping2->ys_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; i<yang_len_get(yspec); i++) 
+	if (yang_cardinality(h, yspec->ys_stmt[i], yang_argument_get(yspec->ys_stmt[i])) < 0)
+	    goto done;
     retval = 0;
  done:
     return retval;

test/lib.sh

@@ -297,11 +297,16 @@ wait_restconf(){
     fi
 }
 
-# Increment test number and print a nice string
+endtest()
-new(){
+{
     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

test/test_feature.sh

@@ -303,4 +303,6 @@ if [ $BE -ne 0 ]; then
     stop_backend -f $cfg
 fi
 
+endtest
+
 rm -rf $dir

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"
+endtest
-# 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
 
 rm -rf $dir

test/test_insert.sh

@@ -214,7 +214,10 @@ testrun "$x0" "<e>32</e>"
 new "adv list add leaf-list"
 testrun "$x0" "<e>32</e>"
 
+endtest
+
 rm -rf $dir
 
 # unset conditional parameters 
 unset clixon_util_xml_mod
+

test/test_nacm_default.sh

@@ -225,6 +225,8 @@ for db in startup init; do
 
 done
 
+endtest
+
 rm -rf $dir
 
 # unset conditional parameters 

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