From 0b3391836f49f51db1513726291d6ef776be0ae1 Mon Sep 17 00:00:00 2001
From: Olof hagsand <olof@hagsand.se>
Date: Thu, 15 Oct 2020 21:35:14 +0200
Subject: [PATCH] * Fixed: Validate/commit error with false positive yang
 choice changes detected in validation found in ietf-ipfix-psamp.yang.

---
 CHANGELOG.md             |  1 +
 lib/src/clixon_xml_map.c | 20 +++++---------------
 test/test_choice.sh      |  2 +-
 3 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2cb0ab41..5656a8c2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -70,6 +70,7 @@ Users may have to change how they access the system
 
 ### Corrected Bugs
 
+* Fixed: Validate/commit error with false positive yang choice changes detected in validation found in ietf-ipfix-psamp.yang.
 * Fixed: Accepted added subtrees containing lists with duplicate keys.
 * Fixed: [default state data returned with get-config](https://github.com/clicon/clixon/issues/140)
   * Generalized default code for both config and state
diff --git a/lib/src/clixon_xml_map.c b/lib/src/clixon_xml_map.c
index 178ad772..220bb3bc 100644
--- a/lib/src/clixon_xml_map.c
+++ b/lib/src/clixon_xml_map.c
@@ -584,21 +584,11 @@ xml_diff1(cxobj     *x0,
 	     * if so, continute compare children but without yang
 	     */
 	    yc = xml_spec(x0c);
-	    if (yc && yang_choice(yc)){
-		/* if x0c and x1c are choice/case, then they are changed */
-		if (cxvec_append(x0c, changed_x0, changedlen) < 0) 
-		    goto done;
-		(*changedlen)--; /* append two vectors */
-		if (cxvec_append(x1c, changed_x1, changedlen) < 0) 
-		    goto done;
-	    }
-	    else if (yc && yang_keyword_get(yc) == Y_LEAF){
-		/* if x0c and x1c are leafs w bodies, then they are changed */
-		if ((b1 = xml_body(x0c)) == NULL) /* empty type */
-		    break;
-		if ((b2 = xml_body(x1c)) == NULL) /* empty type */
-		    break;
-		if (strcmp(b1, b2)){
+	    if (yc && yang_keyword_get(yc) == Y_LEAF){
+		/* if x0c and x1c are leafs w bodies, then they may be changed */
+		if ((b1 = xml_body(x0c)) != NULL && /* skip empty type */
+		    (b2 = xml_body(x1c)) != NULL && /* skip empty type */
+		    strcmp(b1, b2) != 0){
 		    if (cxvec_append(x0c, changed_x0, changedlen) < 0) 
 			goto done;
 		    (*changedlen)--; /* append two vectors */
diff --git a/test/test_choice.sh b/test/test_choice.sh
index d69d80af..9388d33b 100755
--- a/test/test_choice.sh
+++ b/test/test_choice.sh
@@ -181,7 +181,7 @@ new "restconf get protocol tcp"
 expectpart "$(curl $CURLOPTS -X GET $RCPROTO://localhost/restconf/data/system:system)" 0 "HTTP/1.1 200 OK" '{"system:system":{"protocol":{"tcp":\[null\]}}}'
 
 new "restconf set protocol tcp+udp fail again"
-expectpart "$(curl $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/data/system:system/protocol -d '{"system:protocol":{"tcp": [null], "udp": [null]}}')" 0 "HTTP/1.1 400 Bad Request" '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"bad-element","error-info":{"bad-element":"udp"},"error-severity":"error","error-message":"Element in choice statement already exists"}}}'
+expectpart "$(curl $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/data/system:system/protocol -d '{"system:protocol":{"tcp": [null], "udp": [null]}}')" 0 "HTTP/1.1 400 Bad Request" '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"bad-element","error-info":{"bad-element":"tcp"},"error-severity":"error","error-message":"Element in choice statement already exists"}}}'
 
 new "cli set protocol udp"
 expectfn "$clixon_cli -1 -f $cfg -l o set system protocol udp" 0 "^$"