diff --git a/CHANGELOG.md b/CHANGELOG.md index 6922cfd2..313dae7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ * Added libgen.h for baseline() ### Corrected Bugs +* There was a problem with ordered-by-user for XML children that appeared in some circumstances and difficult to trigger. Entries entered by the user did not appear in the order they were entered. This should now be fixed. ## 3.9.0 (21 Feb 2019) diff --git a/docker/system/startsystem.sh b/docker/system/startsystem.sh index 9ba777f6..ed96c9db 100755 --- a/docker/system/startsystem.sh +++ b/docker/system/startsystem.sh @@ -53,7 +53,7 @@ EOF # - test_order.sh XXX this is a bug need debugging cat < /usr/local/bin/test/site.sh # Add your local site specific env variables (or tests) here. -SKIPLIST="test_yangmodels.sh test_openconfig.sh test_install.sh test_order.sh" +SKIPLIST="test_yangmodels.sh test_openconfig.sh test_install.sh" #IETFRFC= EOF diff --git a/lib/clixon/clixon_xml.h b/lib/clixon/clixon_xml.h index 4eff43ca..4775f97b 100644 --- a/lib/clixon/clixon_xml.h +++ b/lib/clixon/clixon_xml.h @@ -142,6 +142,9 @@ int xml_rm(cxobj *xc); int xml_rootchild(cxobj *xp, int i, cxobj **xcp); int xml_rootchild_node(cxobj *xp, cxobj *xc); +int xml_enumerate_children(cxobj *xp); +int xml_enumerate_get(cxobj *x); + char *xml_body(cxobj *xn); cxobj *xml_body_get(cxobj *xn); char *xml_find_type_value(cxobj *xn_parent, char *prefix, diff --git a/lib/src/clixon_log.c b/lib/src/clixon_log.c index 1008210c..8b2d3131 100644 --- a/lib/src/clixon_log.c +++ b/lib/src/clixon_log.c @@ -249,7 +249,7 @@ clicon_log_str(int level, * @code clicon_log(LOG_NOTICE, "%s: dump to dtd not supported", __PROGRAM__); * @endcode - * @see cicon_log_init and clicon_log_str + * @see clicon_log_init and clicon_log_str */ int clicon_log(int level, diff --git a/lib/src/clixon_xml.c b/lib/src/clixon_xml.c index e58e43ab..2c99afb7 100644 --- a/lib/src/clixon_xml.c +++ b/lib/src/clixon_xml.c @@ -122,12 +122,14 @@ struct xml{ int x_childvec_len;/* length of vector */ enum cxobj_type x_type; /* type of node: element, attribute, body */ char *x_value; /* attribute and body nodes have values */ - int _x_vector_i; /* internal use: xml_child_each */ int x_flags; /* Flags according to XML_FLAG_* */ yang_stmt *x_spec; /* Pointer to specification, eg yang, by reference, dont free */ cg_var *x_cv; /* Cached value as cligen variable (eg xml_cmp) */ + int _x_vector_i; /* internal use: xml_child_each */ + int _x_i; /* internal use for sorting: + see xml_enumerate and xml_cmp */ }; /* @@ -1057,6 +1059,43 @@ xml_rootchild_node(cxobj *xp, return retval; } + +/*! help function to sorting: enumerate all children according to present order + * This is so that the child itself know its present order in a list. + * When sorting by "ordered by user", the order should remain in its present + * state. + * A child can always compute its order functionally but it computes + * more cycles,.. + * @param[in] xp Enumerate its children + * @retval 0 OK + * @see xml_sort + * @see xml_enumerate_get Call to the child to get the number + */ +int +xml_enumerate_children(cxobj *xp) +{ + cxobj *x = NULL; + int i = 0; + + while ((x = xml_child_each(xp, x, -1)) != NULL) + x->_x_i = i++; + return 0; +} + +/*! Get the enumeration of a single child set by enumeration of parent + * @see xml_children_enumerate + * @note that it has to be called right after xml_children_enumerate. If not, + * there are many cases where this info is stale. + * @param[in] x A child whose parent has enumerated its children + * @retval n Enumeration + * @see xml_enumerate_children Call to the parent to compute the nr + */ +int +xml_enumerate_get(cxobj *x) +{ + return x->_x_i; +} + /*! Get the first sub-node which is an XML body. * @param[in] xn xml tree node * @retval The returned body as a pointer to the name string @@ -2278,7 +2317,6 @@ clicon_log_xml(int level, return retval; } - /* * Turn this on to get a xml parse and pretty print test program * Usage: xpath diff --git a/lib/src/clixon_xml_sort.c b/lib/src/clixon_xml_sort.c index f9828e11..d258e3cc 100644 --- a/lib/src/clixon_xml_sort.c +++ b/lib/src/clixon_xml_sort.c @@ -194,6 +194,7 @@ xml_child_spec(cxobj *x, * @note args are pointer ot pointers, to fit into qsort cmp function * @see xml_cmp1 Similar, but for one object * @note empty value/NULL is smallest value + * @note xml_enumerate_children must have been called prior to this call */ static int xml_cmp(const void* arg1, @@ -213,12 +214,19 @@ xml_cmp(const void* arg1, char *keyname; cg_var *cv1; cg_var *cv2; + int nr1; + int nr2; - assert(x1&&x2); + if (x1==NULL || x2==NULL) + return 0; /* shouldnt happen */ y1 = xml_spec(x1); y2 = xml_spec(x2); - if (y1==NULL || y2==NULL) - return 0; /* just ignore */ + nr1 = xml_enumerate_get(x1); + nr2 = xml_enumerate_get(x2); + if (y1==NULL || y2==NULL){ + equal = nr1-nr2; + return equal; + } if (y1 != y2){ yi1 = yang_order(y1); yi2 = yang_order(y2); @@ -230,8 +238,10 @@ xml_cmp(const void* arg1, * otherwise sort according to key */ if (yang_config(y1)==0 || - yang_find((yang_node*)y1, Y_ORDERED_BY, "user") != NULL) - return 0; /* Ordered by user or state data : maintain existing order */ + yang_find((yang_node*)y1, Y_ORDERED_BY, "user") != NULL){ + equal = nr1-nr2; + return equal; /* Ordered by user or state data : maintain existing order */ + } switch (y1->ys_keyword){ case Y_LEAF_LIST: /* Match with name and value */ if ((b1 = xml_body(x1)) == NULL) @@ -358,6 +368,7 @@ xml_sort(cxobj *x, /* Abort sort if non-config (=state) data */ if ((ys = xml_spec(x)) != 0 && yang_config(ys)==0) return 1; + xml_enumerate_children(x); qsort(xml_childvec_get(x), xml_child_nr(x), sizeof(cxobj *), xml_cmp); return 0; } @@ -642,6 +653,7 @@ xml_sort_verify(cxobj *x0, retval = 1; goto done; } + xml_enumerate_children(x0); while ((x = xml_child_each(x0, x, -1)) != NULL) { if (xprev != NULL){ /* Check xprev <= x */ if (xml_cmp(&xprev, &x) > 0)