From fc93051b87ef09c53da302ef91c742013b31f17e Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Thu, 28 Feb 2019 17:07:24 +0100 Subject: [PATCH] Fixed numeric ordering of lists (again) [https://github.com/clicon/clixon/issues/64] It was previously just fixed for leaf-lists. --- CHANGELOG.md | 1 + lib/src/clixon_xml_sort.c | 30 +++++++++++++------- test/test_order.sh | 59 +++++++++++++++++++++++++++++++++++---- test/test_perf.sh | 8 +++--- 4 files changed, 79 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fbd4eda4..24ff6a33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ * Added libgen.h for baseline() ### Corrected Bugs +* Fixed numeric ordering of lists (again) [https://github.com/clicon/clixon/issues/64] It was previously just fixed for leaf-lists. * 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/lib/src/clixon_xml_sort.c b/lib/src/clixon_xml_sort.c index d258e3cc..395d6e4a 100644 --- a/lib/src/clixon_xml_sort.c +++ b/lib/src/clixon_xml_sort.c @@ -195,6 +195,7 @@ xml_child_spec(cxobj *x, * @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 + * @note some error cases return as -1 (qsort cant handle errors) */ static int xml_cmp(const void* arg1, @@ -216,22 +217,25 @@ xml_cmp(const void* arg1, cg_var *cv2; int nr1; int nr2; + cxobj *x1b; + cxobj *x2b; - if (x1==NULL || x2==NULL) - return 0; /* shouldnt happen */ + if (x1==NULL || x2==NULL){ + goto done; /* shouldnt happen */ + } y1 = xml_spec(x1); y2 = xml_spec(x2); nr1 = xml_enumerate_get(x1); nr2 = xml_enumerate_get(x2); if (y1==NULL || y2==NULL){ equal = nr1-nr2; - return equal; + goto done; } if (y1 != y2){ yi1 = yang_order(y1); yi2 = yang_order(y2); if ((equal = yi1-yi2) != 0) - return equal; + goto done; } /* Now y1==y2, same Yang spec, can only be list or leaf-list, * But first check exceptions, eg config false or ordered-by user @@ -240,7 +244,7 @@ xml_cmp(const void* arg1, if (yang_config(y1)==0 || yang_find((yang_node*)y1, Y_ORDERED_BY, "user") != NULL){ equal = nr1-nr2; - return equal; /* Ordered by user or state data : maintain existing order */ + goto done; /* Ordered by user or state data : maintain existing order */ } switch (y1->ys_keyword){ case Y_LEAF_LIST: /* Match with name and value */ @@ -249,9 +253,9 @@ xml_cmp(const void* arg1, else if ((b2 = xml_body(x2)) == NULL) equal = 1; else{ - if (xml_cv_cache(x1, b1, &cv1) < 0) + if (xml_cv_cache(x1, b1, &cv1) < 0) /* error case */ goto done; - if (xml_cv_cache(x2, b2, &cv2) < 0) + if (xml_cv_cache(x2, b2, &cv2) < 0) /* error case */ goto done; equal = cv_cmp(cv1, cv2); } @@ -263,12 +267,18 @@ xml_cmp(const void* arg1, cvi = NULL; while ((cvi = cvec_each(cvk, cvi)) != NULL) { keyname = cv_string_get(cvi); /* operational data may have NULL keys*/ - if ((b1 = xml_find_body(x1, keyname)) == NULL) + if ((x1b = xml_find(x1, keyname)) == NULL || + (b1 = xml_body(x1b)) == NULL) equal = -1; - else if ((b2 = xml_find_body(x2, keyname)) == NULL) + else if ((x2b = xml_find(x2, keyname)) == NULL || + (b2 = xml_body(x2b)) == NULL) equal = 1; else{ - if ((equal = strcmp(b1,b2)) != 0) + if (xml_cv_cache(x1b, b1, &cv1) < 0) /* error case */ + goto done; + if (xml_cv_cache(x2b, b2, &cv2) < 0) /* error case */ + goto done; + if ((equal = cv_cmp(cv1, cv2)) != 0) goto done; } } diff --git a/test/test_order.sh b/test/test_order.sh index 14f49746..3a9fc7ba 100755 --- a/test/test_order.sh +++ b/test/test_order.sh @@ -96,12 +96,28 @@ module order-example{ type int32; ordered-by system; } + list listints{ + ordered-by system; + key a; + leaf a { + type int32; + } + } leaf-list decs{ type decimal64{ fraction-digits 3; } ordered-by system; } + list listdecs{ + ordered-by system; + key a; + leaf a { + type decimal64{ + fraction-digits 3; + } + } + } } } EOF @@ -204,17 +220,50 @@ new "verify list user order (as entered)" expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 ']]>]]>' '^cbarbfooafie]]>]]>$' #-- order by type rather than strings. -# there are three lists: strings, ints, and decimal64 +# there are three leaf-lists:strings, ints, and decimal64, and two lists: +# listints and listdecs # the strings is there for comparison -new "add type ordered entries" +# The check is to write the entries as: 10,2,1, and then expect them to +# get back as 1,2,10 (if typed). +new "put strings (10,2,1)" +expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 ' +1021 +]]>]]>' "^]]>]]>$" + +new "check string order (1,10,2)" +expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 ']]>]]>' '^1102]]>]]>$' + +new "put leaf-list int (10,2,1)" expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 ' 1021 -1021 +]]>]]>' "^]]>]]>$" + +new "check leaf-list int order (1,2,10)" +expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 ']]>]]>' '^1210]]>]]>$' + +new "put list int (10,2,1)" +expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 ' +1021 +]]>]]>' "^]]>]]>$" + +new "check list int order (1,2,10)" +expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 ']]>]]>' '^1210]]>]]>$' + +new "put leaf-list decimal64 (10,2,1)" +expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 ' 10.02.01.0 ]]>]]>' "^]]>]]>$" -new "get type ordered entries" -expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 ']]>]]>' '^110212101.02.010.0]]>]]>$' +new "check leaf-list decimal64 order (1,2,10)" +expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 ']]>]]>' '^1.02.010.0]]>]]>$' + +new "put list decimal64 (10,2,1)" +expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 ' +10.02.01.0 +]]>]]>' "^]]>]]>$" + +new "check list decimal64 order (1,2,10)" +expecteof "$clixon_netconf -qf $cfg -y $fyang" 0 ']]>]]>' '^1.02.010.0]]>]]>$' if [ $BE -eq 0 ]; then exit # BE diff --git a/test/test_perf.sh b/test/test_perf.sh index 89985e0b..2bb07d15 100755 --- a/test/test_perf.sh +++ b/test/test_perf.sh @@ -25,10 +25,10 @@ module scaling{ list y { key "a"; leaf a { - type string; + type int32; } leaf b { - type string; + type int32; } } leaf-list c { @@ -131,10 +131,10 @@ done # Instead of many small entries, get one large in netconf and restconf new "netconf get large config" -expecteof "/usr/bin/time -f %e $clixon_netconf -qf $cfg -y $fyang" 0 "]]>]]>" '^0011' +expecteof "/usr/bin/time -f %e $clixon_netconf -qf $cfg -y $fyang" 0 "]]>]]>" '^00112233' new "restconf get large config" -expecteof "/usr/bin/time -f %e curl -sG http://localhost/restconf/data" 0 "]]>]]>" '^{"data": {"scaling:x": {"y": \[{"a": "0","b": "0"},{ "a": "1","b": "1"},' +expecteof "/usr/bin/time -f %e curl -sG http://localhost/restconf/data" 0 "]]>]]>" '^{"data": {"scaling:x": {"y": \[{"a": 0,"b": 0},{ "a": 1,"b": 1},{ "a": 2,"b": 2},{ "a": 3,"b": 3},' # Now do leaf-lists istead of leafs