Fixed: [Error with submodules and feature Interaction](clicon/clixon-…

…controller#158)

CHANGELOG.md

@@ -37,6 +37,7 @@ Developers may need to change their code
 
 ### Corrected Bugs
 
+* Fixed: [Error with submodules and feature Interaction](https://github.com/clicon/clixon-controller/issues/158)
 * Fixed: [Expansion removes the double quote](https://github.com/clicon/clixon/issues/524)
   *  Add escaping in expand_dbvar instead of automatic in cligen expand
 * Fixed: [string length validation doesn't work for the entry "" in case it has default value specified](https://github.com/clicon/clixon/issues/563)

lib/src/clixon_yang.c

@@ -3613,7 +3613,7 @@ yang_features(clixon_handle h,
     yang_stmt *ys = NULL;
     yang_stmt *ymod;
     const char *mainfile = NULL;
-    int        ret;
+    int        enabled;
 
     i = 0;
     while (i<yt->ys_len){
@@ -3622,16 +3622,16 @@ yang_features(clixon_handle h,
             /* Parse the if-feature-expr string using yang sub-parser */
             if ((ymod = ys_module(ys)) != NULL)
                 mainfile = yang_filename_get(ymod);
-            ret = 0;
-            if (yang_subparse(yang_argument_get(ys), ys, YA_IF_FEATURE, mainfile, 1, &ret, h) < 0)
+            enabled = 0;
+            if (yang_subparse(h, yang_argument_get(ys), ys, YA_IF_FEATURE, mainfile, 1, &enabled) < 0)
                 goto done;
-            clixon_debug(CLIXON_DBG_YANG | CLIXON_DBG_DETAIL, "%s %d", yang_argument_get(ys), ret);
-            if (ret == 0)
+            clixon_debug(CLIXON_DBG_YANG | CLIXON_DBG_DETAIL, "%s %d", yang_argument_get(ys), enabled);
+            if (enabled == 0)
                 goto disabled;
         }
         else if (ys->ys_keyword == Y_FEATURE){
-                if (ys_populate_feature(h, ys) < 0)
-                    goto done;
+            if (ys_populate_feature(h, ys) < 0)
+                goto done;
         }
         else
             switch (yang_features(h, ys)){

lib/src/clixon_yang_parse_lib.c

@@ -2060,7 +2060,7 @@ ys_parse_sub(yang_stmt  *ys,
          * pass 1 is not yet resolved, only check syntax, actual feature check made in next pass
          * @see yang_features
          */
-        if (yang_subparse(yang_argument_get(ys), ys, YA_IF_FEATURE, filename, yang_linenum_get(ys), NULL, NULL) < 0)
+        if (yang_subparse(NULL, yang_argument_get(ys), ys, YA_IF_FEATURE, filename, yang_linenum_get(ys), NULL) < 0)
             goto done;
         break;
     case Y_AUGMENT: /* If parent is module/submodule: absolute-schema-nodeid

lib/src/clixon_yang_sub_parse.c

@@ -61,24 +61,24 @@
 
 /*! Invoke yang sub-parser on string
  *
- * @param[in]  str           yang string
- * @param[in]  ys            Yang statement
- * @param[in]  accept        Sub-parse rule to accept
- * @param[in]  mainfile      Name of main parse file
- * @param[in]  linenum       Line number context in mainfile (assume parse module of ys)
- * @param[in]  h             Clixon handle
- * @param[out] enabled       0: Disabled, 1: Enabled (if present)
- * @retval     0             OK
- * @retval    -1             Error
+ * @param[in]  h         Clixon handle (can be NULL)
+ * @param[in]  str       Yang string
+ * @param[in]  ys        Yang statement
+ * @param[in]  accept    Sub-parse rule to accept
+ * @param[in]  mainfile  Name of main parse file
+ * @param[in]  linenum   Line number context in mainfile (assume parse module of ys)
+ * @param[out] enabled   0: Disabled, 1: Enabled (if present)
+ * @retval     0         OK
+ * @retval    -1         Error
  */
 int
-yang_subparse(char                      *str,
+yang_subparse(clixon_handle              h,
+              char                      *str,
               yang_stmt                 *ys,
               enum yang_sub_parse_accept accept,
               const char                *mainfile,
               int                        linenum,
-              int                       *enabled,
-              clixon_handle             h)
+              int                       *enabled)
 {
     int                    retval = -1;
     clixon_yang_sub_parse_yacc ife = {0,};
@@ -90,7 +90,7 @@ yang_subparse(char                      *str,
         ife.if_ys = ys; /* Used as trigger to check if enabled */
     ife.if_accept = accept;
     ife.if_mainfile = mainfile;
-    ife.h = h;
+    ife.if_h = h;
     if (clixon_yang_sub_parsel_init(&ife) < 0)
         goto done;
     if (clixon_yang_sub_parseparse(&ife) != 0) { /* yacc returns 1 on error */

lib/src/clixon_yang_sub_parse.h

@@ -49,14 +49,14 @@ enum yang_sub_parse_accept{
 
 /*! XML parser yacc handler struct */
 struct clixon_yang_sub_parse_yacc {
-    char       *if_parse_string; /* original (copy of) parse string */
-    const char *if_mainfile;     /* Original main-file (this is a sib-parser) */
-    int         if_linenum;      /* Number of \n in parsed buffer (in mainfile) */
-    void      *if_lexbuf;       /* Internal parse buffer from lex */
-    yang_stmt *if_ys;           /* Yang statement, NULL if no check */
-    enum yang_sub_parse_accept if_accept; /* Which sub-parse rule to accept */
-    int         if_enabled;      /* Result: 0: feature disabled, 1: enabled */
-    clixon_handle h;
+    char                      *if_parse_string; /* original (copy of) parse string */
+    const char                *if_mainfile;     /* Original main-file (this is a sib-parser) */
+    int                        if_linenum;      /* Number of \n in parsed buffer (in mainfile) */
+    void                      *if_lexbuf;       /* Internal parse buffer from lex */
+    yang_stmt                 *if_ys;           /* Yang statement, NULL if no check */
+    enum yang_sub_parse_accept if_accept;       /* Which sub-parse rule to accept */
+    int                        if_enabled;      /* Result: 0: feature disabled, 1: enabled */
+    clixon_handle              if_h;
 };
 typedef struct clixon_yang_sub_parse_yacc clixon_yang_sub_parse_yacc;
 
@@ -74,7 +74,7 @@ int clixon_yang_sub_parsel_linenr(void);
 int clixon_yang_sub_parselex(void *);
 int clixon_yang_sub_parseparse(void *);
 
-int  yang_subparse(char *str, yang_stmt *ys, enum yang_sub_parse_accept accept, const char *mainfile, int linenum, int *enabled, clixon_handle h);
+int  yang_subparse(clixon_handle h, char *str, yang_stmt *ys, enum yang_sub_parse_accept accept, const char *mainfile, int linenum, int *enabled);
 int  yang_schema_nodeid_subparse(char *str, enum yang_sub_parse_accept accept, const char *mainfile, int linenum);
 
 #endif  /* _CLIXON_YANG_SUB_PARSER_H_ */

lib/src/clixon_yang_sub_parse.y

@@ -86,6 +86,8 @@
 #include <stdlib.h>
 #include <sys/time.h>
 
+#include <assert.h> // XXX
+
 /* cligen */
 #include <cligen/cligen.h>
 
@@ -128,10 +130,11 @@ clixon_yang_sub_parseerror(void *arg,
 
 /*! Check if feature "str" is enabled or not in context of yang node ys
  *
+ * @param[in]  ife  If-feature struct
  * @param[in]  str  feature str. 
- * @param[in]  ys   If-feature type yang node
  * @retval     0    OK
  * @retval    -1    Error
+ * @note ife->if_ys is used as implicit flag to perform actual checks
  */
 static int
 if_feature_check(clixon_yang_sub_parse_yacc *ife,
@@ -150,12 +153,19 @@ if_feature_check(clixon_yang_sub_parse_yacc *ife,
     if (nodeid_split(str, &prefix, &feature) < 0)
         goto done;
     /* Specifically need to handle? strcmp(prefix, myprefix)) */
-    if (prefix == NULL)
-        ymod = ys_module(ys);
-    else
+    if (prefix != NULL){
         ymod = yang_find_module_by_prefix(ys, prefix);
-    if (ymod == NULL)
+        if (ys_real_module(ymod, &ymod) < 0)
+            goto done;
+    }
+    else {
+        if (ys_real_module(ys, &ymod) < 0)
+            goto done;
+    }
+    if (ymod == NULL){
+        clixon_err(OE_YANG, 0, "Module not found: %s", str);
         goto done;
+    }
     /* Check if feature exists, and is set, otherwise remove */
     if ((yfeat = yang_find(ymod, Y_FEATURE, feature)) == NULL){
         clixon_err(OE_YANG, EINVAL, "Yang module %s has IF_FEATURE %s, but no such FEATURE statement exists",
@@ -167,8 +177,8 @@ if_feature_check(clixon_yang_sub_parse_yacc *ife,
      * Continue loop to catch unbound features and make verdict at end
      */
     cv = yang_cv_get(yfeat);
-    if (cv == NULL && ife->h) {
-        ys_populate_feature(ife->h, yfeat);
+    if (cv == NULL && ife->if_h) {
+        ys_populate_feature(ife->if_h, yfeat);
     }
 
     cv = yang_cv_get(yfeat);

test/test_feature.sh

@@ -23,7 +23,8 @@ s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi
 APPNAME=example
 
 cfg=$dir/conf_yang.xml
-fyang=$dir/test.yang
+fyang=$dir/example.yang
+fsub1=$dir/sub1.yang
 
 # Note ietf-routing@2018-03-13 assumed
 cat <<EOF > $cfg
@@ -34,6 +35,7 @@ cat <<EOF > $cfg
   <CLICON_CONFIGFILE>$cfg</CLICON_CONFIGFILE>
   <CLICON_YANG_DIR>${YANG_INSTALLDIR}</CLICON_YANG_DIR>
   <CLICON_YANG_DIR>$IETFRFC</CLICON_YANG_DIR>
+  <CLICON_YANG_DIR>${dir}</CLICON_YANG_DIR>
   <CLICON_YANG_MAIN_FILE>$fyang</CLICON_YANG_MAIN_FILE>
   <CLICON_CLISPEC_DIR>/usr/local/lib/$APPNAME/clispec</CLICON_CLISPEC_DIR>
   <CLICON_CLI_DIR>/usr/local/lib/$APPNAME/cli</CLICON_CLI_DIR>
@@ -45,11 +47,36 @@ cat <<EOF > $cfg
 </clixon-config>
 EOF
 
+cat <<EOF > $fsub1
+submodule sub1{
+   yang-version 1.1;
+   belongs-to example {
+      prefix ex;
+   }
+   feature S1{
+      description "submodule feature";
+   }
+   leaf subA1{
+     if-feature A;
+     type "string";
+   }
+   leaf subA2{
+     if-feature ex:A;
+     type "string";
+   }
+   leaf subS1{
+     if-feature S1;
+     type "string";
+   }
+}
+EOF
+
 cat <<EOF > $fyang
 module example{
    yang-version 1.1;
    namespace "urn:example:clixon";
    prefix ex;
+   include sub1;
    import ietf-routing {
         prefix rt;
    }
@@ -76,18 +103,18 @@ module example{
    leaf u {
     if-feature rt:router-id;
     type "string";
-   } 
+   }
    leaf z{
      type "string";
    }
    leaf m1{
-     if-feature "A and 
+     if-feature "A and
         A1";
      description "Enabled";
      type "string";
    }
    leaf m2{
-     if-feature "A or 
+     if-feature "A or
         A1";
      description "Enabled";
      type "string";
@@ -139,6 +166,10 @@ module example{
      description "Disabled";
      type "string";
    }
+   leaf subS1{
+     if-feature S1;
+     type "string";
+   }
 }
 EOF
 
@@ -147,12 +178,12 @@ EOF
 # 2: disabled or enabled
 # NOTE, this was before failures when disabled, but after https://github.com/clicon/clixon/issues/322 that
 # disabled nodes should be "ignored". Instead now if disabled a random node is inserted under the disabled node
-# which should not work 
+# which should not work
 function testrun()
 {
     node=$1
     enabled=$2
-    
+
     new "netconf set $node"
     expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><$node xmlns=\"urn:example:clixon\">foo</$node></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
 
@@ -227,7 +258,7 @@ testrun m11 false
 # reply since the modules change so often
 new "netconf schema resource, RFC 7895"
 rpc=$(chunked_framing "<rpc $DEFAULTNS><get><filter type=\"xpath\" select=\"l:yang-library/l:module-set[l:name='default']/l:module\" xmlns:l=\"urn:ietf:params:xml:ns:yang:ietf-yang-library\"/></get></rpc>")
-ret=$($clixon_netconf -qf $cfg<<EOF 
+ret=$($clixon_netconf -qf $cfg<<EOF
 $DEFAULTHELLO$rpc
 EOF
    )
@@ -241,13 +272,13 @@ if [ -z "$match" ]; then
 fi
 
 new "netconf module A"
-expect="<module><name>example</name><namespace>urn:example:clixon</namespace><feature>A</feature><feature>A1</feature></module>"
+expect="<module><name>example</name><namespace>urn:example:clixon</namespace><submodule><name>sub1</name></submodule><feature>A</feature><feature>A1</feature></module>"
 match=`echo "$ret" | grep --null -Go "$expect"`
 if [ -z "$match" ]; then
       err "$expect" "$ret"
 fi
 
-if false ; then # clixon "config" is a meta-config and not visisble in regular features
+if false ; then # clixon "config" is a meta-config and not visible in regular features
 new "netconf module clixon-config"
 expect="<module><name>clixon-config</name><revision>2018-09-30</revision><namespace/></module>"
 match=`echo "$ret" | grep --null -Go "$expect"`
@@ -308,8 +339,17 @@ if [ $BE -ne 0 ]; then
     stop_backend -f $cfg
 fi
 
+new "test params: -f $cfg"
+if [ $BE -ne 0 ]; then
+    new "kill old backend"
+    sudo clixon_backend -zf $cfg
+    if [ $? -ne 0 ]; then
+        err
+    fi
+fi
+
 #------------------------
-# Negative test, if if-feature but no feature, signal error
+# Negative test 1, if if-feature but no feature, signal error
 cat <<EOF > $fyang
 module example{
    yang-version 1.1;
@@ -324,17 +364,53 @@ module example{
    }
 }
 EOF
+if [ $BE -ne 0 ]; then
+    new "Feature B missing expected fail"
+    expectpart "$(sudo $clixon_backend -F1s init -f $cfg -l o)" 255 " Yang module example has IF_FEATURE B, but no such FEATURE statement exists"
 
-new "test params: -f $cfg"
+    stop_backend -f $cfg
+fi
+
+# Negative test 2, if if-feature but no feature, signal error
+cat <<EOF > $fyang
+module example{
+   yang-version 1.1;
+   namespace "urn:example:clixon";
+   prefix ex;
+   include sub1;
+   leaf x{
+     if-feature "S2";
+     type "string";
+   }
+}
+EOF
 if [ $BE -ne 0 ]; then
-    new "kill old backend"
-    sudo clixon_backend -zf $cfg
-    if [ $? -ne 0 ]; then
-        err
-    fi
+    new "Feature S2 missing expected fail"
+    expectpart "$(sudo $clixon_backend -F1s init -f $cfg -l o)" 255 " Yang module example has IF_FEATURE S2, but no such FEATURE statement exists"
 
-    new "start backend -s init -f $cfg: feature missing expected fail"
-    expectpart "$(sudo $clixon_backend -F1s init -f $cfg -l o)" 255 " Yang module example has IF_FEATURE B, but no such FEATURE statement exists"
+    stop_backend -f $cfg
+fi
+
+# Negative test 3, if if-feature but no feature, signal error
+cat <<EOF > $fsub1
+submodule sub1{
+   yang-version 1.1;
+   belongs-to example {
+      prefix ex;
+   }
+   feature S2{
+      description "submodule feature";
+   }
+   leaf x{
+     if-feature "A";
+     type "string";
+   }
+}
+EOF
+
+if [ $BE -ne 0 ]; then
+    new "Feature A missing expected fail"
+    expectpart "$(sudo $clixon_backend -F1s init -f $cfg -l o)" 255 " Yang module example has IF_FEATURE A, but no such FEATURE statement exists"
 
     stop_backend -f $cfg
 fi