diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6158dbd3..447b1563 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -52,6 +52,7 @@ Expected: September 2022
### Corrected Bugs
+* Fixed: [YANG ordering fails for nested choice and action](YANG ordering fails for nested choice and action)
* Fixed: [YANG min-elements within non-presence container does not work](https://github.com/clicon/clixon/issues/355)
* Fixed: [Issues with ietf-snmp modules](https://github.com/clicon/clixon/issues/353)
* Fixed: [Missing/no namespace error in YANG augments with default values](https://github.com/clicon/clixon/issues/354)
diff --git a/lib/src/clixon_xml_sort.c b/lib/src/clixon_xml_sort.c
index 3082b835..065f9696 100644
--- a/lib/src/clixon_xml_sort.c
+++ b/lib/src/clixon_xml_sort.c
@@ -182,19 +182,18 @@ xml_cv_cache_clear(cxobj *xt)
* Example: x1: 1 and x2: 12 are considered equal.
* This is useful in searching for indexes in trees, where x1 is a search index and not a
* complete tree.
- *
- * @note empty value/NULL is smallest value
- * @note some error cases return as -1 (qsort cant handle errors)
- * @note some error cases return as -1 (qsort cant handle errors)
- *
- * @note "comparing" x1 and x2 here judges equality from a structural (model)
+ *
+ * "Comparing" x1 and x2 here determines equality from a structural (model)
* perspective, ie both have the same yang spec, if they are lists, they have the
* the same keys. NOT that the values are equal!
- * In other words, if x is a leaf with the same yang spec, 1 and 2 are
- * "equal".
+ * In other words, if x is a leaf with the same yang spec, 1 and 2 are "equal".
* If x is a list element (with key "k"),
- * 42foo and 42bar are equal,
+ * 42foo and 42bar are equal,
* but is not equal to 71bar
+ *
+ * @note empty value/NULL is smallest value
+ * @note some error cases return as -1 (qsort cant handle errors) (which?)
+ * - yang_order() is one
*/
int
xml_cmp(cxobj *x1,
@@ -256,6 +255,7 @@ xml_cmp(cxobj *x1,
goto done;
}
if (y1 != y2){
+ /* XXX handle errors */
yi1 = yang_order(y1);
yi2 = yang_order(y2);
if ((equal = yi1-yi2) != 0)
@@ -388,7 +388,7 @@ xml_cmp(cxobj *x1,
}
/*!
- * @note args are pointer ot pointers, to fit into qsort cmp function
+ * @note args are pointer to pointers, to fit into qsort cmp function
*/
static int
xml_cmp_qsort(const void* arg1,
@@ -476,11 +476,14 @@ xml_find_keys_notsorted(cxobj *xp,
int i;
cxobj *xc;
yang_stmt *yc;
+ int yi;
for (i=mid+1; i=0; i--){ /* Then decrement */
xc = xml_child_i(xp, i);
yc = xml_spec(xc);
- if (yangi != yang_order(yc)) /* wrong yang */
+ if ((yi = yang_order(yc)) < -1)
+ goto done;
+ if (yangi != yi) /* wrong yang */
break;
if (xml_cmp(xc, x1, 0, skip1, NULL) == 0){
if (clixon_xvec_append(xvec, xc) < 0)
@@ -528,11 +533,14 @@ search_multi_equals(cxobj **childvec,
int i;
cxobj *xc;
yang_stmt *yc;
+ int yi;
for (i=mid-1; i>=0; i--){ /* First decrement */
xc = childvec[i];
yc = xml_spec(xc);
- if (yangi != yang_order(yc)) /* wrong yang */
+ if ((yi = yang_order(yc)) < -1)
+ goto done;
+ if (yangi != yi) /* wrong yang */
break;
if (xml_cmp(x1, xc, 0, skip1, NULL) != 0)
break;
@@ -542,7 +550,9 @@ search_multi_equals(cxobj **childvec,
for (i=mid+1; i=0; i--){ /* First decrement */
xc = clixon_xvec_i(childvec, i);
yc = xml_spec(xc);
- if (yangi != yang_order(yc)) /* wrong yang */
+ if ((yi = yang_order(yc)) < -1)
+ goto done;
+ if (yangi != yi) /* wrong yang */
break;
if (xml_cmp(x1, xc, 0, skip1, NULL) != 0)
break;
@@ -583,7 +596,9 @@ search_multi_equals_xvec(clixon_xvec *childvec,
for (i=mid+1; i upper){ /* beyond range */
clicon_err(OE_XML, 0, "low>upper %d %d", low, upper);
@@ -994,7 +1013,9 @@ xml_insert2(cxobj *xp,
cmp = xml_cmp(xn, xc, 0, 0, NULL);
}
else{ /* Not equal yang - compute diff */
- cmp = yni - yang_order(yc);
+ if ((yi = yang_order(yc)) < -1)
+ goto done;
+ cmp = yni - yi;
/* One case is a choice where
* xc = , xn =
* same order but different yang spec
@@ -1072,7 +1093,8 @@ xml_insert(cxobj *xp,
#endif
if (yang_keyword_get(y) == Y_LIST || yang_keyword_get(y) == Y_LEAF_LIST)
userorder = (yang_find(y, Y_ORDERED_BY, "user") != NULL);
- yi = yang_order(y);
+ if ((yi = yang_order(y)) < -1)
+ goto done;
if ((i = xml_insert2(xp, xi, y, yi,
userorder, ins, key_val, nsc_key,
low, upper)) < 0)
diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c
index 20033bd7..5e47ed26 100644
--- a/lib/src/clixon_yang.c
+++ b/lib/src/clixon_yang.c
@@ -1466,40 +1466,46 @@ yang_choice(yang_stmt *y)
* In (2) we increment with only 1.
*/
static int
-order1_choice(yang_stmt *yp,
- yang_stmt *y,
- int *index)
+yang_order1_choice(yang_stmt *yp,
+ yang_stmt *y,
+ int *index)
{
yang_stmt *ys;
yang_stmt *yc;
int i;
int j;
- int shortcut=0;
int max=0;
+ int index0;
+ index0 = *index;
for (i=0; iys_len; i++){ /* Loop through choice */
ys = yp->ys_stmt[i];
if (ys->ys_keyword == Y_CASE){ /* Loop through case */
+ *index = index0;
for (j=0; jys_len; j++){
yc = ys->ys_stmt[j];
- if (yang_datanode(yc) && yc == y){
- *index += j;
- return 1;
+ if (yc->ys_keyword == Y_CHOICE){
+ if (yang_order1_choice(yc, y, index) == 1) /* If one of the choices is "y" */
+ return 1;
+ }
+ else {
+ if (yang_datanode(yc) && yc == y){
+ // *index = index0 + j;
+ return 1;
+ }
+ (*index)++;
}
}
- if (j>max)
- max = j;
+ if (*index-index0 > max)
+ max = *index-index0;
}
else {
- shortcut = 1; /* Shortcut, no case */
+ max = 1; /* Shortcut, no case */
if (yang_datanode(ys) && ys == y)
return 1;
}
}
- if (shortcut)
- (*index)++;
- else
- *index += max;
+ *index += max;
return 0;
}
@@ -1507,44 +1513,52 @@ order1_choice(yang_stmt *yp,
* @param[in] yp Parent
* @param[in] y Yang datanode to find
* @param[out] index Index of y in yp:s list of children
- * @retval 0 not found (must be datanode)
- * @retval 1 found
+ * @retval 0 OK: found
+ * @retval -1 Error: not found (must be datanode)
*/
static int
-order1(yang_stmt *yp,
- yang_stmt *y,
- int *index)
+yang_order1(yang_stmt *yp,
+ yang_stmt *y,
+ int *index)
{
+ int retval = -1;
yang_stmt *ys;
int i;
for (i=0; iys_len; i++){
ys = yp->ys_stmt[i];
if (ys->ys_keyword == Y_CHOICE){
- if (order1_choice(ys, y, index) == 1) /* If one of the choices is "y" */
- return 1;
+ if (yang_order1_choice(ys, y, index) == 1) /* If one of the choices is "y" */
+ break;
}
else {
- if (!yang_datanode(ys))
+ if (!yang_datanode(ys) &&
+ yang_keyword_get(ys) != Y_ACTION) /* action is special case */
continue;
if (ys == y)
- return 1;
+ break;
(*index)++;
}
}
- return 0;
+ if (i < yp->ys_len) /* break -> found */
+ retval = 0;
+ else
+ assert(0); // XXX
+ return retval;
}
/*! Return order of yang statement y in parents child vector
* @param[in] y Find position of this data-node
* @param[out] index Index of y in yp:s list of children
* @retval >=0 Order of child with specified argument
- * @retval -1 Not found
+ * @retval -1 No spec, y is NULL, which applies to eg attributes and are placed first
+ * @retval -2 Error: Not found
* @note special handling if y is child of (sub)module
*/
int
yang_order(yang_stmt *y)
{
+ int retval = -2;
yang_stmt *yp;
yang_stmt *ypp;
yang_stmt *ym;
@@ -1552,9 +1566,10 @@ yang_order(yang_stmt *y)
int j=0;
int tot = 0;
- if (y == NULL)
- return -1;
-
+ if (y == NULL){
+ retval = -1;
+ goto done;
+ }
/* Some special handling if yp is choice (or case)
* if so, the real parent (from an xml point of view) is the parents
* parent.
@@ -1578,9 +1593,13 @@ yang_order(yang_stmt *y)
tot += ym->ys_len;
}
}
- if (order1(yp, y, &j) == 1)
- return tot + j;
- return -1;
+ if (yang_order1(yp, y, &j) < 0){
+ clicon_err(OE_YANG, 0, "YANG node %s not ordered: not found", yang_argument_get(y));
+ goto done;
+ }
+ retval = tot + j;
+ done:
+ return retval;
}
char *
diff --git a/test/test_augment_default.sh b/test/test_augment_default.sh
index 0a62f02e..b1b33bb7 100755
--- a/test/test_augment_default.sh
+++ b/test/test_augment_default.sh
@@ -89,21 +89,18 @@ new "wait backend"
wait_backend
new "cli show config startup"
-#expectpart "$($clixon_cli -1 -f $cfg -l o show config xml)" 0 '