experiments/clixon
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fixed: [show compare does not show correct diff while load merge xml](https://github.com/clicon/clixon-controller/issues/101)

Browse source 
Diff code did not check non-yang leaf/terminal values
This commit is contained in:
Olof hagsand 2024-02-08 10:41:13 +01:00
parent 840233d6fa
commit 4ee6b48abe
4 changed files with 133 additions and 76 deletions
Download patch file Download diff file Expand all files Collapse all files

1
CHANGELOG.md
View file

@ -90,6 +90,7 @@ Developers may need to change their code
### Corrected Bugs ### Corrected Bugs
* Fixed: [show compare does not show correct diff while load merge xml](https://github.com/clicon/clixon-controller/issues/101)
* Fixed: [commit goes 2 times](https://github.com/clicon/clixon/issues/488) * Fixed: [commit goes 2 times](https://github.com/clicon/clixon/issues/488)
* Fixed: Problem with cl:ignore attribute for show compare * Fixed: Problem with cl:ignore attribute for show compare
* Fixed: [yang_enum_int_value() fails if no explicit values are assigned to enums](https://github.com/clicon/clixon/issues/483) * Fixed: [yang_enum_int_value() fails if no explicit values are assigned to enums](https://github.com/clicon/clixon/issues/483)

55
lib/src/clixon_text_syntax.c
View file

@ -669,8 +669,8 @@ text_diff2cbuf(cbuf *cb,
int retval = -1; int retval = -1;
cxobj *x0c = NULL; /* x0 child */ cxobj *x0c = NULL; /* x0 child */
cxobj *x1c = NULL; /* x1 child */ cxobj *x1c = NULL; /* x1 child */
yang_stmt *yc0; yang_stmt *y0c;
yang_stmt *yc1; yang_stmt *y1c;
char *b0; char *b0;
char *b1; char *b1;
int eq; int eq;
@ -699,19 +699,19 @@ text_diff2cbuf(cbuf *cb,
if (x0c == NULL && x1c == NULL) if (x0c == NULL && x1c == NULL)
goto ok; goto ok;
else { else {
yc0 = NULL; y0c = NULL;
yc1 = NULL; y1c = NULL;
/* If cl:ignore-compare extension, skip */ /* If cl:ignore-compare extension, skip */
if (x0c && (yc0 = xml_spec(x0c)) != NULL){ if (x0c && (y0c = xml_spec(x0c)) != NULL){
if (yang_extension_value(yc0, "ignore-compare", CLIXON_LIB_NS, &extflag, NULL) < 0) if (yang_extension_value(y0c, "ignore-compare", CLIXON_LIB_NS, &extflag, NULL) < 0)
goto done; goto done;
if (extflag){ /* skip */ if (extflag){ /* skip */
x0c = xml_child_each(x0, x0c, CX_ELMNT); x0c = xml_child_each(x0, x0c, CX_ELMNT);
continue; continue;
} }
} }
if (x1c && (yc1 = xml_spec(x1c)) != NULL){ if (x1c && (y1c = xml_spec(x1c)) != NULL){
if (yang_extension_value(yc1, "ignore-compare", CLIXON_LIB_NS, &extflag, NULL) < 0) if (yang_extension_value(y1c, "ignore-compare", CLIXON_LIB_NS, &extflag, NULL) < 0)
goto done; goto done;
if (extflag){ /* skip */ if (extflag){ /* skip */
x1c = xml_child_each(x1, x1c, CX_ELMNT); x1c = xml_child_each(x1, x1c, CX_ELMNT);
@ -751,8 +751,10 @@ text_diff2cbuf(cbuf *cb,
} }
/* Both x0c and x1c exists, check if they are yang-equal. */ /* Both x0c and x1c exists, check if they are yang-equal. */
eq = xml_cmp(x0c, x1c, 0, 0, NULL); eq = xml_cmp(x0c, x1c, 0, 0, NULL);
if (eq && yc0 && yc1 && yc0 == yc1 && yang_find(yc0, Y_ORDERED_BY, "user")){ b0 = xml_body(x0c);
if (text_diff2cbuf_ordered_by_user(cb, x0, x1, x0c, x1c, yc0, b1 = xml_body(x1c);
if (eq && y0c && y1c && y0c == y1c && yang_find(y0c, Y_ORDERED_BY, "user")){
if (text_diff2cbuf_ordered_by_user(cb, x0, x1, x0c, x1c, y0c,
level, skiptop) < 0) level, skiptop) < 0)
goto done; goto done;
/* Add all in x0 marked as DELETE in x0vec /* Add all in x0 marked as DELETE in x0vec
@ -776,7 +778,7 @@ text_diff2cbuf(cbuf *cb,
} }
} }
while ((xi = xml_child_each(x0, xi, CX_ELMNT)) != NULL && while ((xi = xml_child_each(x0, xi, CX_ELMNT)) != NULL &&
xml_spec(xi) == yc0); xml_spec(xi) == y0c);
x0c = xi; x0c = xi;
/* Add all in x1 marked as ADD in x1vec */ /* Add all in x1 marked as ADD in x1vec */
@ -798,7 +800,7 @@ text_diff2cbuf(cbuf *cb,
} }
} }
while ((xj = xml_child_each(x1, xj, CX_ELMNT)) != NULL && while ((xj = xml_child_each(x1, xj, CX_ELMNT)) != NULL &&
xml_spec(xj) == yc1); xml_spec(xj) == y1c);
x1c = xj; x1c = xj;
continue; continue;
} }
@ -833,7 +835,7 @@ text_diff2cbuf(cbuf *cb,
continue; continue;
} }
else{ /* equal */ else{ /* equal */
if (yc0 && yc1 && yc0 != yc1){ /* choice */ if (y0c && y1c && y0c != y1c){ /* choice */
if (nr==0 && skiptop==0){ if (nr==0 && skiptop==0){
cprintf(cb, "%*s", level1, ""); cprintf(cb, "%*s", level1, "");
if (prefix) if (prefix)
@ -846,13 +848,10 @@ text_diff2cbuf(cbuf *cb,
if (text2cbuf(cb, x1c, level+1, "+", 0, &leafl, &leaflname) < 0) if (text2cbuf(cb, x1c, level+1, "+", 0, &leafl, &leaflname) < 0)
goto done; goto done;
} }
else if (yc0 && yang_keyword_get(yc0) == Y_LEAF){ else if (y0c && yang_keyword_get(y0c) == Y_LEAF){
b0 = xml_body(x0c);
b1 = xml_body(x1c);
if (b0 == NULL && b1 == NULL) if (b0 == NULL && b1 == NULL)
; ;
else if (b0 == NULL || b1 == NULL else if (b0 == NULL || b1 == NULL || strcmp(b0, b1) != 0) {
|| strcmp(b0, b1) != 0){
if (nr==0 && skiptop == 0){ if (nr==0 && skiptop == 0){
cprintf(cb, "%*s", level1, ""); cprintf(cb, "%*s", level1, "");
if (prefix) if (prefix)
@ -862,8 +861,24 @@ text_diff2cbuf(cbuf *cb,
cprintf(cb, " {\n"); cprintf(cb, " {\n");
nr++; nr++;
} }
cprintf(cb, "-%*s%s %s;\n", level1+PRETTYPRINT_INDENT-1, "", xml_name(x0c), b0); cprintf(cb, "-%*s%s \"%s\";\n", level1+PRETTYPRINT_INDENT-1, "", xml_name(x0c), b0);
cprintf(cb, "+%*s%s %s;\n", level1+PRETTYPRINT_INDENT-1, "", xml_name(x1c), b1); cprintf(cb, "+%*s%s \"%s\";\n", level1+PRETTYPRINT_INDENT-1, "", xml_name(x1c), b1);
}
}
else if (y0c == NULL && y1c == NULL && (b0 || b1)) {
/* Special case for anydata terminals */
if (b0 == NULL || b1 == NULL || strcmp(b0, b1) != 0) {
if (nr==0 && skiptop == 0){
cprintf(cb, "%*s", level1, "");
if (prefix)
cprintf(cb, "%s:", prefix);
cprintf(cb, "%s", xml_name(x0));
text_diff_keys(cb, x0, y0);
cprintf(cb, " {\n");
nr++;
}
cprintf(cb, "-%*s%s \"%s\";\n", level1+PRETTYPRINT_INDENT-1, "", xml_name(x0c), b0);
cprintf(cb, "+%*s%s \"%s\";\n", level1+PRETTYPRINT_INDENT-1, "", xml_name(x1c), b1);
} }
} }
else if (text_diff2cbuf(cb, x0c, x1c, level+1, 0) < 0) else if (text_diff2cbuf(cb, x0c, x1c, level+1, 0) < 0)

145
lib/src/clixon_xml_io.c
View file

@ -1259,6 +1259,72 @@ xml_diff2cbuf_ordered_by_user(cbuf *cb,
return retval; return retval;
} }
/*! xml_diff2cbuf helper function to compute leaf difference
*
* @param[out] cb CLIgen buffer
* @param[in] x0 First XML tree
* @param[in] x1 Second XML tree
* @param[in] level How many spaces to insert before each line
* @param[in] skiptop 0: Include top object 1: Skip top-object, only children,
* @retval 0 Ok
* @retval -1 Error
*/
static int
xml_diff2cbuf_leaf(cbuf *cb,
cxobj *x0,
cxobj *x1,
int level,
int skiptop,
yang_stmt *y0,
cxobj *x0c,
cxobj *x1c,
char *b0,
char *b1,
int *nr)
{
int retval = -1;
char *e0 = NULL;
char *e1 = NULL;
if (*nr==0 && skiptop==0){
xml_diff_context(cb, x0, level*PRETTYPRINT_INDENT);
xml_diff_keys(cb, x0, y0, (level+1)*PRETTYPRINT_INDENT);
(*nr)++;
}
/* Encode data to XML */
if (b0){
if (xml_chardata_encode(&e0, "%s", b0) < 0)
goto done;
}
else
e0 = NULL;
if (b1){
if (xml_chardata_encode(&e1, "%s", b1) < 0)
goto done;
}
else
e1 = NULL;
cprintf(cb, "-%*s%s>%s</%s>\n", ((level+1)*PRETTYPRINT_INDENT-1), "<",
xml_name(x0c), e0, xml_name(x0c));
cprintf(cb, "+%*s%s>%s</%s>\n", ((level+1)*PRETTYPRINT_INDENT-1), "<",
xml_name(x1c), e1, xml_name(x1c));
if (e0){
free(e0);
e0 = NULL;
}
if (e1){
free(e1);
e1 = NULL;
}
retval = 0;
done:
if (e0)
free(e0);
if (e1)
free(e1);
return retval;
}
/*! Print XML diff of two cxobj trees into a cbuf /*! Print XML diff of two cxobj trees into a cbuf
* *
* YANG dependent * YANG dependent
@ -1270,7 +1336,7 @@ xml_diff2cbuf_ordered_by_user(cbuf *cb,
* @param[in] skiptop 0: Include top object 1: Skip top-object, only children, * @param[in] skiptop 0: Include top object 1: Skip top-object, only children,
* @retval 0 Ok * @retval 0 Ok
* @retval -1 Error * @retval -1 Error
* @cod * @code
* cbuf *cb = cbuf_new(); * cbuf *cb = cbuf_new();
* if (clixon_xml_diff2cbuf(cb, 0, x0, x1) < 0) * if (clixon_xml_diff2cbuf(cb, 0, x0, x1) < 0)
* err(); * err();
@ -1293,12 +1359,10 @@ xml_diff2cbuf(cbuf *cb,
cxobj *x0c = NULL; /* x0 child */ cxobj *x0c = NULL; /* x0 child */
cxobj *x1c = NULL; /* x1 child */ cxobj *x1c = NULL; /* x1 child */
yang_stmt *y0; yang_stmt *y0;
yang_stmt *yc0; yang_stmt *y0c;
yang_stmt *yc1; yang_stmt *y1c;
char *b0; char *b0;
char *b1; char *b1;
char *e0 = NULL;
char *e1 = NULL;
int eq; int eq;
int nr=0; int nr=0;
int level1; int level1;
@ -1316,19 +1380,19 @@ xml_diff2cbuf(cbuf *cb,
if (x0c == NULL && x1c == NULL) if (x0c == NULL && x1c == NULL)
goto ok; goto ok;
else { else {
yc0 = NULL; y0c = NULL;
yc1 = NULL; y1c = NULL;
/* If cl:ignore-compare extension, skip */ /* If cl:ignore-compare extension, skip */
if (x0c && (yc0 = xml_spec(x0c)) != NULL){ if (x0c && (y0c = xml_spec(x0c)) != NULL){
if (yang_extension_value(yc0, "ignore-compare", CLIXON_LIB_NS, &extflag, NULL) < 0) if (yang_extension_value(y0c, "ignore-compare", CLIXON_LIB_NS, &extflag, NULL) < 0)
goto done; goto done;
if (extflag){ /* skip */ if (extflag){ /* skip */
x0c = xml_child_each(x0, x0c, CX_ELMNT); x0c = xml_child_each(x0, x0c, CX_ELMNT);
continue; continue;
} }
} }
if (x1c && (yc1 = xml_spec(x1c)) != NULL){ if (x1c && (y1c = xml_spec(x1c)) != NULL){
if (yang_extension_value(yc1, "ignore-compare", CLIXON_LIB_NS, &extflag, NULL) < 0) if (yang_extension_value(y1c, "ignore-compare", CLIXON_LIB_NS, &extflag, NULL) < 0)
goto done; goto done;
if (extflag){ /* skip */ if (extflag){ /* skip */
x1c = xml_child_each(x1, x1c, CX_ELMNT); x1c = xml_child_each(x1, x1c, CX_ELMNT);
@ -1361,8 +1425,10 @@ xml_diff2cbuf(cbuf *cb,
} }
/* Both x0c and x1c exists, check if yang equal */ /* Both x0c and x1c exists, check if yang equal */
eq = xml_cmp(x0c, x1c, 0, 0, NULL); eq = xml_cmp(x0c, x1c, 0, 0, NULL);
if (eq && yc0 && yc1 && yc0 == yc1 && yang_find(yc0, Y_ORDERED_BY, "user")){ b0 = xml_body(x0c);
if (xml_diff2cbuf_ordered_by_user(cb, x0, x1, x0c, x1c, yc0, b1 = xml_body(x1c);
if (eq && y0c && y1c && y0c == y1c && yang_find(y0c, Y_ORDERED_BY, "user")){
if (xml_diff2cbuf_ordered_by_user(cb, x0, x1, x0c, x1c, y0c,
level, skiptop) < 0) level, skiptop) < 0)
goto done; goto done;
/* Add all in x0 marked as DELETE in x0vec /* Add all in x0 marked as DELETE in x0vec
@ -1382,7 +1448,7 @@ xml_diff2cbuf(cbuf *cb,
} }
} }
while ((xi = xml_child_each(x0, xi, CX_ELMNT)) != NULL && while ((xi = xml_child_each(x0, xi, CX_ELMNT)) != NULL &&
xml_spec(xi) == yc0); xml_spec(xi) == y0c);
x0c = xi; x0c = xi;
/* Add all in x1 marked as ADD in x1vec */ /* Add all in x1 marked as ADD in x1vec */
@ -1400,7 +1466,7 @@ xml_diff2cbuf(cbuf *cb,
} }
} }
while ((xj = xml_child_each(x1, xj, CX_ELMNT)) != NULL && while ((xj = xml_child_each(x1, xj, CX_ELMNT)) != NULL &&
xml_spec(xj) == yc1); xml_spec(xj) == y1c);
x1c = xj; x1c = xj;
continue; continue;
} }
@ -1430,7 +1496,7 @@ xml_diff2cbuf(cbuf *cb,
/* xml-spec NULL could happen with anydata children for example, /* xml-spec NULL could happen with anydata children for example,
* if so, continute compare children but without yang * if so, continute compare children but without yang
*/ */
if (yc0 && yc1 && yc0 != yc1){ /* choice */ if (y0c && y1c && y0c != y1c){ /* choice */
if (nr==0 && skiptop==0){ if (nr==0 && skiptop==0){
xml_diff_context(cb, x0, level1); xml_diff_context(cb, x0, level1);
xml_diff_keys(cb, x0, y0, (level+1)*PRETTYPRINT_INDENT); xml_diff_keys(cb, x0, y0, (level+1)*PRETTYPRINT_INDENT);
@ -1441,48 +1507,27 @@ xml_diff2cbuf(cbuf *cb,
if (clixon_xml2cbuf(cb, x1c, level+1, 1, "+", -1, 0) < 0) if (clixon_xml2cbuf(cb, x1c, level+1, 1, "+", -1, 0) < 0)
goto done; goto done;
} }
else if (yc0 && yang_keyword_get(yc0) == Y_LEAF){ else if (y0c && yang_keyword_get(y0c) == Y_LEAF){
/* if x0c and x1c are leafs w bodies, then they may be changed */ /* if x0c and x1c are leafs w bodies, then they may be changed */
b0 = xml_body(x0c);
b1 = xml_body(x1c);
if (b0 == NULL && b1 == NULL) if (b0 == NULL && b1 == NULL)
; ;
else if (b0 == NULL || b1 == NULL else if (b0 == NULL || b1 == NULL || strcmp(b0, b1) != 0){
|| strcmp(b0, b1) != 0){ if (xml_diff2cbuf_leaf(cb, x0, x1, level, skiptop,
if (nr==0 && skiptop==0){ y0, x0c, x1c, b0, b1, &nr) < 0)
xml_diff_context(cb, x0, level1);
xml_diff_keys(cb, x0, y0, (level+1)*PRETTYPRINT_INDENT);
nr++;
}
/* Encode data to XML */
if (b0){
if (xml_chardata_encode(&e0, "%s", b0) < 0)
goto done; goto done;
} }
else }
e0 = NULL; else if (y0c == NULL && y1c == NULL && (b0 || b1)) {
if (b1){ /* Special case for anydata terminals */
if (xml_chardata_encode(&e1, "%s", b1) < 0) if (b0 == NULL || b1 == NULL || strcmp(b0, b1) != 0){
if (xml_diff2cbuf_leaf(cb, x0, x1, level, skiptop,
y0, x0c, x1c, b0, b1, &nr) < 0)
goto done; goto done;
} }
else
e1 = NULL;
cprintf(cb, "-%*s%s>%s</%s>\n", ((level+1)*PRETTYPRINT_INDENT-1), "<",
xml_name(x0c), e0, xml_name(x0c));
cprintf(cb, "+%*s%s>%s</%s>\n", ((level+1)*PRETTYPRINT_INDENT-1), "<",
xml_name(x1c), e1, xml_name(x1c));
if (e0){
free(e0);
e0 = NULL;
}
if (e1){
free(e1);
e1 = NULL;
}
}
} }
else if (xml_diff2cbuf(cb, x0c, x1c, level+1, 0) < 0) else if (xml_diff2cbuf(cb, x0c, x1c, level+1, 0) < 0)
goto done; goto done;
} }
/* Get next */ /* Get next */
x0c = xml_child_each(x0, x0c, CX_ELMNT); x0c = xml_child_each(x0, x0c, CX_ELMNT);
@ -1493,10 +1538,6 @@ xml_diff2cbuf(cbuf *cb,
cprintf(cb, "%*s</%s>\n", level*PRETTYPRINT_INDENT, "", xml_name(x0)); cprintf(cb, "%*s</%s>\n", level*PRETTYPRINT_INDENT, "", xml_name(x0));
retval = 0; retval = 0;
done: done:
if (e0)
free(e0);
if (e1)
free(e1);
return retval; return retval;
} }

2
test/test_cli_diff.sh
View file

@ -170,7 +170,7 @@ echo "$clixon_cli -1 -f $cfg show compare xml"
expectpart "$($clixon_cli -1 -f $cfg show compare xml)" 0 "<table xmlns=\"urn:example:clixon\">" "^\-\ *<parameter>" "^+\ *<parameter>" "^\-\ *<name>a</name>" "^+\ *<name>c</name>" --not-- "^+\ *<name>a</name>" "^\-\ *<name>c</name>" expectpart "$($clixon_cli -1 -f $cfg show compare xml)" 0 "<table xmlns=\"urn:example:clixon\">" "^\-\ *<parameter>" "^+\ *<parameter>" "^\-\ *<name>a</name>" "^+\ *<name>c</name>" --not-- "^+\ *<name>a</name>" "^\-\ *<name>c</name>"
new "check compare text" new "check compare text"
expectpart "$($clixon_cli -1 -f $cfg show compare text)" 0 "^\ *table {" "^\-\ *parameter a {" "^+\ *parameter c {" "^\-\ *value 98;" "^+\ *value 99;" expectpart "$($clixon_cli -1 -f $cfg show compare text)" 0 "^\ *table {" "^\-\ *parameter a {" "^+\ *parameter c {" "^\-\ *value \"98\";" "^+\ *value \"99\";"
new "delete section x" new "delete section x"
expectpart "$($clixon_cli -1 -f $cfg delete top section x)" 0 "^$" expectpart "$($clixon_cli -1 -f $cfg delete top section x)" 0 "^$"