diff --git a/CHANGELOG.md b/CHANGELOG.md index 627f204d..919d820f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,8 @@ Developers may need to change their code ### Corrected Bugs +* Fixed: RFC 8040 yang-data extension allows non-key lists + * Added YANG_FLAG_NOKEY as exception to mandatory key lists * Fixed: mandatory leaf in a uses statement caused abort * Occurence was in ietf-yang-patch.yang * Native RESTCONF fixes for http/1 or http/2 only modes diff --git a/apps/restconf/restconf_lib.c b/apps/restconf/restconf_lib.c index 9b0592c4..1f5c28c0 100644 --- a/apps/restconf/restconf_lib.c +++ b/apps/restconf/restconf_lib.c @@ -499,6 +499,14 @@ restconf_insert_attributes(cxobj *xdata, * @param[in] ys Yang node of (unknown) statement belonging to extension * @retval 0 OK, all callbacks executed OK * @retval -1 Error in one callback + * @note This extension adds semantics to YANG according to RFC8040 as follows: + * - The list-stmt is not required to have a key-stmt defined.(NB!!) + * - The if-feature-stmt is ignored if present. + * - The config-stmt is ignored if present. + * - The available identity values for any 'identityref' + * leaf or leaf-list nodes are limited to the module + * containing this extension statement and the modules + * imported into that module. */ int restconf_main_extension_cb(clicon_handle h, @@ -522,6 +530,9 @@ restconf_main_extension_cb(clicon_handle h, goto ok; if ((yn = ys_dup(yc)) == NULL) goto done; + /* yang-data extension: The list-stmt is not required to have a key-stmt defined. + */ + yang_flag_set(yn, YANG_FLAG_NOKEY); if (yn_insert(yang_parent_get(ys), yn) < 0) goto done; ok: diff --git a/lib/clixon/clixon_yang.h b/lib/clixon/clixon_yang.h index 8e482cad..437318a7 100644 --- a/lib/clixon/clixon_yang.h +++ b/lib/clixon/clixon_yang.h @@ -53,8 +53,12 @@ */ #define YANG_FLAG_MARK 0x01 /* (Dynamic) marker for dynamic algorithms, eg expand and DAG */ #define YANG_FLAG_TMP 0x02 /* (Dynamic) marker for dynamic algorithms, eg DAG detection */ +#define YANG_FLAG_NOKEY 0x04 /* Key not mandatory in this list, see eg yang-data extension in + * RFC 8040 / ietf-restconf.yang + * see restconf_main_extension_cb + */ #ifdef XML_EXPLICIT_INDEX -#define YANG_FLAG_INDEX 0x04 /* This yang node under list is (extra) index. --> you can access +#define YANG_FLAG_INDEX 0x08 /* This yang node under list is (extra) index. --> you can access * list elements using this index with binary search */ #endif diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c index 056dee6d..7c1af5a1 100644 --- a/lib/src/clixon_yang.c +++ b/lib/src/clixon_yang.c @@ -259,11 +259,15 @@ yang_cv_get(yang_stmt *ys) /*! Set yang statement CLIgen variable * @param[in] ys Yang statement node * @param[in] cv cligen variable + * @note: Frees on replace, not if cv is NULL. This is for some ys_cp/ys_dup cases, which means + * you need to free it explicitly to set it to NULL proper. */ int yang_cv_set(yang_stmt *ys, cg_var *cv) { + if (cv != NULL && ys->ys_cv != NULL) + cv_free(ys->ys_cv); ys->ys_cv = cv; return 0; } @@ -469,13 +473,14 @@ ys_free1(yang_stmt *ys, int self) { cg_var *cv; + if (ys->ys_argument){ free(ys->ys_argument); ys->ys_argument = NULL; } if ((cv = yang_cv_get(ys)) != NULL){ + yang_cv_set(ys, NULL); /* only frees on replace */ cv_free(cv); - yang_cv_set(ys, NULL); } if (ys->ys_cvec){ cvec_free(ys->ys_cvec); @@ -629,6 +634,7 @@ ys_cp(yang_stmt *ynew, clicon_err(OE_YANG, errno, "strdup"); goto done; } + yang_cv_set(ynew, NULL); if ((cvo = yang_cv_get(yold)) != NULL){ if ((cvn = cv_dup(cvo)) == NULL){ clicon_err(OE_YANG, errno, "cv_dup"); @@ -1469,7 +1475,8 @@ ys_real_module(yang_stmt *ys, * @param[in] ys Any yang statement in a yang tree * @retval yspec The top yang specification * @see ys_module - * @see yang_augment_node where shortcut is set + * @see yang_augment_node where shortcut is set for augment + * @see yang_myroot for first node under (sub)module */ yang_stmt * ys_spec(yang_stmt *ys) diff --git a/lib/src/clixon_yang_parse_lib.c b/lib/src/clixon_yang_parse_lib.c index 2548363b..c472f292 100644 --- a/lib/src/clixon_yang_parse_lib.c +++ b/lib/src/clixon_yang_parse_lib.c @@ -633,13 +633,16 @@ yang_expand_uses_node(yang_stmt *yn, yang_argument_get(yg), yang_argument_get(ygrouping) ); - goto done; + goto done; } if (yang_when_xpath_set(yg, wxpath) < 0) goto done; if (yang_when_nsc_set(yg, wnsc) < 0) goto done; } + /* This is for extensions that allow list keys to be optional, see restconf_main_extension_cb */ + if (yang_flag_get(ys, YANG_FLAG_NOKEY)) + yang_flag_set(yg, YANG_FLAG_NOKEY); yn->ys_stmt[i+k] = yg; yg->ys_parent = yn; k++; @@ -1196,7 +1199,7 @@ ys_schemanode_check(yang_stmt *ys, * Verify the following rule: * RFC 7950 7.8.2: The "key" statement, which MUST be present if the list represents * configuration and MAY be present otherwise - * Unless CLICON_YANG_LIST_CHECK is false + * Unless CLICON_YANG_LIST_CHECK is false (obsolete) * OR it is the "errors" rule of the ietf-restconf spec which seems to be a special case. */ static int @@ -1222,28 +1225,22 @@ ys_list_check(clicon_handle h, if (keyw == Y_LIST && yang_find(ys, Y_KEY, NULL) == 0){ ymod = ys_module(ys); -#if 1 - /* Except restconf error extension from sanity check, dont know why it has no keys */ - if (strcmp(yang_find_mynamespace(ys),"urn:ietf:params:xml:ns:yang:ietf-restconf")==0 && - strcmp(yang_argument_get(ys),"error") == 0) - ; - else -#endif - { - if (clicon_option_bool(h, "CLICON_YANG_LIST_CHECK")){ - clicon_log(LOG_ERR, "Error: LIST \"%s\" in module \"%s\" lacks key statement which MUST be present (See RFC 7950 Sec 7.8.2)", - yang_argument_get(ys), - yang_argument_get(ymod) - ); - - goto done; - } - else - clicon_log(LOG_WARNING, "Warning: LIST \"%s\" in module \"%s\" lacks key statement which MUST be present (See RFC 7950 Sec 7.8.2)", + /* Except nokey exceptions such as rrc 8040 yang-data */ + if (!yang_flag_get(yroot, YANG_FLAG_NOKEY)){ + /* Note obsolete */ + if (clicon_option_bool(h, "CLICON_YANG_LIST_CHECK")){ + clicon_log(LOG_ERR, "Error: LIST \"%s\" in module \"%s\" lacks key statement which MUST be present (See RFC 7950 Sec 7.8.2)", yang_argument_get(ys), yang_argument_get(ymod) ); + goto done; } + else + clicon_log(LOG_WARNING, "Warning: LIST \"%s\" in module \"%s\" lacks key statement which MUST be present (See RFC 7950 Sec 7.8.2)", + yang_argument_get(ys), + yang_argument_get(ymod) + ); + } } /* Traverse subs */ if (yang_schemanode(ys) || keyw == Y_MODULE || keyw == Y_SUBMODULE){ diff --git a/test/mem.sh b/test/mem.sh index 42e2667e..ed493d78 100755 --- a/test/mem.sh +++ b/test/mem.sh @@ -40,7 +40,7 @@ function memonce(){ sudo chmod 660 $valgrindfile sudo chown www-data $valgrindfile : ${DEMWAIT:=15} # valgrind backend needs some time to get up - clixon_restconf="/usr/bin/valgrind --leak-check=full --show-leak-kinds=all --suppressions=./valgrind-clixon.supp --track-fds=yes --trace-children=no --child-silent-after-fork=yes --log-file=$valgrindfile clixon_restconf" + clixon_restconf="/usr/bin/valgrind --num-callers=50 --leak-check=full --show-leak-kinds=all --suppressions=./valgrind-clixon.supp --track-fds=yes --trace-children=no --child-silent-after-fork=yes --log-file=$valgrindfile clixon_restconf" ;; *)