- Refactoring of get_common/get_list_pagination with two sub-functions

- Fixed memory leak

CHANGELOG.md

@@ -35,7 +35,7 @@ Expected: September, 2021
 
 ### New features
 
-* List pageing for Netconf and Restconf
+* List paging for Netconf and Restconf
   * Experimental, work-in-progress
     * Enable with LIST_PAGINATION compile-time option
   * According to:
@@ -773,7 +773,7 @@ Developers may need to change their code
   * The register function has removed `from` and `rev` parameters: `upgrade_callback_register(h, cb, namespace, arg)`
   * The callback function has a new `op` parameter with possible values: `XML_FLAG_ADD`, `XML_FLAG_CHANGE` or `XML_FLAG_CHANGE`: `clicon_upgrade_cb(h, xn, ns, op, from, to, arg, cbret)`
 
-* Added new cli show functions to work with cligen_output for cligen pageing to work. To achieve this, replace function calls as follows:
+* Added new cli show functions to work with cligen_output for cligen paging to work. To achieve this, replace function calls as follows:
   * `xml2txt(...)` --> `xml2txt_cb(..., cligen_output)`
   * `xml2cli(...)` --> `xml2cli_cb(..., cligen_output)`
   * `clicon_xml2file(...)` --> `clicon_xml2file_cb(..., cligen_output)`

apps/backend/backend_client.c

@@ -150,6 +150,8 @@ release_all_dbs(clicon_handle h,
     }
     retval = 0;
  done:
+    if (keys)
+	free(keys);
     return retval;
 }
 

apps/backend/backend_get.c

@@ -73,9 +73,6 @@
 #include "backend_handle.h"
 #include "backend_get.h"
 
-/* List pagination remaining attribute using meta */
-#undef REMAINING
-
 /*!
  * Maybe should be in the restconf client instead of backend?
  * @param[in]     h       Clicon handle
@@ -276,6 +273,122 @@ get_client_statedata(clicon_handle h,
     goto done;
 }
 
+/*! Help function to filter out anything that is outside of xpath 
+ *
+ * Code complex to filter out anything that is outside of xpath 
+ * Actually this is a safety catch, should really be done in plugins
+ * and modules_state functions.
+ * But it is problematic, because defaults, at least of config data, is in place
+ * and we need to re-add it.
+ * Note original xpath
+ *
+ * @param[in]  h       Clicon handle 
+ * @param[in]  yspec   Yang spec
+ * @param[in]  xret    Result XML tree
+ * @param[in]  xpath   XPath point to object to get
+ * @param[in]  nsc     Namespace context of xpath
+ * @param[out] x1p     Pointer to first matching object if any
+ * @retval     0       OK
+ * @retval    -1       Error
+ */
+static int
+filter_xpath_again(clicon_handle h,
+		   yang_stmt    *yspec,
+		   cxobj        *xret,
+		   char         *xpath,
+		   cvec         *nsc,
+		   cxobj       **x1p)
+{
+    int     retval = -1;
+    cxobj **xvec = NULL;
+    size_t  xlen;
+    int     i;
+
+    if (xpath_vec(xret, nsc, "%s", &xvec, &xlen, xpath?xpath:"/") < 0)
+	goto done;
+    /* If vectors are specified then mark the nodes found and
+     * then filter out everything else,
+     * otherwise return complete tree.
+     */
+    if (xvec != NULL){
+	for (i=0; i<xlen; i++)
+	    xml_flag_set(xvec[i], XML_FLAG_MARK);
+    }
+    /* Remove everything that is not marked */
+    if (!xml_flag(xret, XML_FLAG_MARK))
+	if (xml_tree_prune_flagged_sub(xret, XML_FLAG_MARK, 1, NULL) < 0)
+	    goto done;
+    /* reset flag */
+    if (xml_apply(xret, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, (void*)XML_FLAG_MARK) < 0)
+	goto done;
+    /* Add default global values */
+    if (xml_global_defaults(h, xret, nsc, xpath, yspec, 0) < 0)
+	goto done;
+    /* Add default recursive values */
+    if (xml_default_recurse(xret, 0) < 0)
+	goto done;
+    if (xlen && x1p)
+	*x1p = xvec[0];
+    retval = 0;
+ done:
+    if (xvec)
+	free(xvec);
+    return retval;
+}
+
+/*! Help function for NACM access and returnmessage
+ *
+ * @param[in]  h        Clicon handle 
+ * @param[in]  xret     Result XML tree
+ * @param[in]  xpath    XPath point to object to get
+ * @param[in]  nsc      Namespace context of xpath
+ * @param[in]  username User name for NACM access
+ * @param[in]  depth    Nr of levels to print, -1 is all, 0 is none
+ * @param[out] cbret    Return xml tree, eg <rpc-reply>..., <rpc-error.. 
+ * @retval     0        OK
+ * @retval    -1        Error
+ */
+static int
+get_nacm_and_reply(clicon_handle h,
+		   cxobj        *xret,
+		   char         *xpath,
+		   cvec         *nsc,
+		   char         *username,
+		   int32_t       depth,
+		   cbuf         *cbret)
+{
+    int     retval = -1;
+    cxobj **xvec = NULL;
+    size_t  xlen;
+    cxobj  *xnacm = NULL;
+
+    /* Pre-NACM access step */
+    xnacm = clicon_nacm_cache(h);
+    if (xnacm != NULL){ /* Do NACM validation */
+	if (xpath_vec(xret, nsc, "%s", &xvec, &xlen, xpath?xpath:"/") < 0)
+	    goto done;
+	/* NACM datanode/module read validation */
+	if (nacm_datanode_read(h, xret, xvec, xlen, username, xnacm) < 0) 
+	    goto done;
+    }
+    cprintf(cbret, "<rpc-reply xmlns=\"%s\">", NETCONF_BASE_NAMESPACE);     /* OK */
+    if (xret==NULL)
+	cprintf(cbret, "<data/>");
+    else{
+	if (xml_name_set(xret, NETCONF_OUTPUT_DATA) < 0)
+	    goto done;
+	/* Top level is data, so add 1 to depth if significant */
+	if (clicon_xml2cbuf(cbret, xret, 0, 0, depth>0?depth+1:depth) < 0)
+	    goto done;
+    }
+    cprintf(cbret, "</rpc-reply>");
+    retval = 0;
+ done:
+    if (xvec)
+	free(xvec);
+    return retval;
+}
+
 #ifdef LIST_PAGINATION
 
 /*! Help function for parsing restconf query parameter and setting netconf attribute
@@ -318,8 +431,8 @@ element2value(clicon_handle  h,
  * @param[in]  xe      Request: <rpc><xn></rpc> 
  * @param[in]  content Get config/state/both
  * @param[in]  db      Database name
- * @param[in]  xpath
+ * @param[in]  xpath   XPath point to object to get
- * @param[in]  nsc
+ * @param[in]  nsc     Namespace context of xpath
  * @param[out] cbret   Return xml tree, eg <rpc-reply>..., <rpc-error.. 
  * @retval     0       OK
  * @retval    -1       Error
@@ -351,20 +464,15 @@ get_list_pagination(clicon_handle        h,
     cxobj          *x;
     int             list_config;
     yang_stmt      *ylist;
-    cxobj         **xvec = NULL;
-    size_t          xlen;
-    cxobj         **xvecnacm = NULL;
-    size_t          xlennacm;    
     cxobj          *xerr = NULL;
     cbuf           *cbmsg = NULL; /* For error msg */
     cxobj          *xret = NULL;
-    char           *xpath2; /* With optional pageing predicate */
+    char           *xpath2; /* With optional paging predicate */
     int             ret;
-    int             i;
-    cxobj          *xnacm = NULL;
     uint32_t        iddb; /* DBs lock, if any */
     enum paging_status pagingstatus;
-#ifdef REMAINING
+    cxobj          *x1 = NULL;
+#ifdef LIST_PAGINATION_REMAINING
     cxobj          *xcache = NULL;
     uint32_t        total = 0;
     uint32_t        remaining = 0;
@@ -458,7 +566,7 @@ get_list_pagination(clicon_handle        h,
 	cprintf(cbpath, "[position() < %u]", limit);
     /* Append predicate to original xpath and replace it */
     xpath2 = cbuf_get(cbpath);
-#ifdef REMAINING
+#ifdef LIST_PAGINATION_REMAINING
     /* Get total/remaining
      * XXX: Works only for cache, and only if already populated
      * XXX: Maybe together with get config / state data
@@ -477,7 +585,7 @@ get_list_pagination(clicon_handle        h,
 	 * 2. Read all here and count (fallback)
 	 */
     }
-#endif /* REMAINING */
+#endif /* LIST_PAGINATION_REMAINING */
     /* Read config */
     switch (content){
     case CONTENT_CONFIG:    /* config data only */
@@ -512,49 +620,18 @@ get_list_pagination(clicon_handle        h,
 					       offset, limit, &xret)) < 0)
 	    goto done;
     }
-    /* Code complex to filter out anything that is outside of xpath 
+    if (filter_xpath_again(h, yspec, xret, xpath, nsc, &x1) < 0)
-     * Actually this is a safety catch, should really be done in plugins
-     * and modules_state functions.
-     * But it is problematic, because defaults, at least of config data, is in place
-     * and we need to re-add it.
-     * Note original xpath
-     */
-    if (xpath_vec(xret, nsc, "%s", &xvec, &xlen, xpath?xpath:"/") < 0)
 	goto done;
-
+#ifdef LIST_PAGINATION_REMAINING
-    /* If vectors are specified then mark the nodes found and
-     * then filter out everything else,
-     * otherwise return complete tree.
-     */
-    if (xvec != NULL){
-	for (i=0; i<xlen; i++)
-	    xml_flag_set(xvec[i], XML_FLAG_MARK);
-    }
-    /* Remove everything that is not marked */
-    if (!xml_flag(xret, XML_FLAG_MARK))
-	if (xml_tree_prune_flagged_sub(xret, XML_FLAG_MARK, 1, NULL) < 0)
-	    goto done;
-    /* reset flag */
-    if (xml_apply(xret, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, (void*)XML_FLAG_MARK) < 0)
-	goto done;
-    /* Add default global values */
-    if (xml_global_defaults(h, xret, nsc, xpath, yspec, 0) < 0)
-	goto done;
-    /* Add default recursive values */
-    if (xml_default_recurse(xret, 0) < 0)
-	goto done;
-
-#ifdef REMAINING
     /* Add remaining attribute Sec 3.1.5: 
        Any list or leaf-list that is limited includes, on the first element in the result set, 
        a metadata value [RFC7952] called "remaining"*/
-    if (limit && xlen){ 
+    if (limit && x1){ 
 	cxobj *xa;
 	cbuf  *cba = NULL;
 
 	/* Add remaining attribute */
-	x = xvec[0];
+	if ((xa = xml_new("remaining", x1, CX_ATTR)) == NULL)
-	if ((xa = xml_new("remaining", x, CX_ATTR)) == NULL)
 	    goto done;
 	if ((cba = cbuf_new()) == NULL){
 	    clicon_err(OE_UNIX, errno, "cbuf_new");
@@ -565,32 +642,14 @@ get_list_pagination(clicon_handle        h,
 	    goto done;
 	if (xml_prefix_set(xa, "cp") < 0)
 	    goto done;
-	if (xmlns_set(x, "cp", "http://clicon.org/clixon-netconf-list-pagination") < 0)
+	if (xmlns_set(x1, "cp", "http://clicon.org/clixon-netconf-list-pagination") < 0)
 	    goto done;
 	if (cba)
 	    cbuf_free(cba);
     }
-#endif /* REMAINING */
+#endif /* LIST_PAGINATION_REMAINING */
-    /* Pre-NACM access step */
+    if (get_nacm_and_reply(h, xret, xpath, nsc, username, depth, cbret) < 0)
-    xnacm = clicon_nacm_cache(h);
-    if (xnacm != NULL){ /* Do NACM validation */
-	if (xpath_vec(xret, nsc, "%s", &xvecnacm, &xlennacm, xpath?xpath:"/") < 0)
 	goto done;
-	/* NACM datanode/module read validation */
-	if (nacm_datanode_read(h, xret, xvecnacm, xlennacm, username, xnacm) < 0) 
-	    goto done;
-    }
-    cprintf(cbret, "<rpc-reply xmlns=\"%s\">", NETCONF_BASE_NAMESPACE);     /* OK */
-    if (xret==NULL)
-	cprintf(cbret, "<data/>");
-    else{
-	if (xml_name_set(xret, NETCONF_OUTPUT_DATA) < 0)
-	    goto done;
-	/* Top level is data, so add 1 to depth if significant */
-	if (clicon_xml2cbuf(cbret, xret, 0, 0, depth>0?depth+1:depth) < 0)
-	    goto done;
-    }
-    cprintf(cbret, "</rpc-reply>");
  ok:
     retval = 0;
  done:
@@ -600,10 +659,6 @@ get_list_pagination(clicon_handle        h,
 	cbuf_free(cbpath);
     if (xerr)
 	xml_free(xerr);
-    if (xvec)
-	free(xvec);
-    if (xvecnacm)
-	free(xvecnacm);
     if (xret)
 	xml_free(xret);
     return retval;
@@ -636,18 +691,12 @@ get_common(clicon_handle        h,
     cxobj          *xfilter;
     char           *xpath = NULL;
     cxobj          *xret = NULL;
-    cxobj         **xvec = NULL;
-    size_t          xlen;
-    cxobj         **xvecnacm = NULL;
-    size_t          xlennacm;    
-    cxobj          *xnacm = NULL;
     char           *username;
     cvec           *nsc0 = NULL; /* Create a netconf namespace context from filter */
     cvec           *nsc = NULL;
     char           *attr;
     int32_t         depth = -1; /* Nr of levels to print, -1 is all, 0 is none */
     yang_stmt      *yspec;
-    int             i;
     cxobj          *xerr = NULL;
     int             ret;
     char           *reason = NULL;
@@ -817,57 +866,10 @@ get_common(clicon_handle        h,
 	if (xml_apply(xret, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, (void*)XML_FLAG_MARK) < 0)
 	    goto done;
     }
-    /* Code complex to filter out anything that is outside of xpath 
+    if (filter_xpath_again(h, yspec, xret, xpath, nsc, NULL) < 0)
-     * Actually this is a safety catch, should really be done in plugins
-     * and modules_state functions.
-     * But it is problematic, because defaults, at least of config data, is in place
-     * and we need to re-add it.
-     * Note original xpath
-     */
-    if (xpath_vec(xret, nsc, "%s", &xvec, &xlen, xpath?xpath:"/") < 0)
 	goto done;
-
+    if (get_nacm_and_reply(h, xret, xpath, nsc, username, depth, cbret) < 0)
-    /* If vectors are specified then mark the nodes found and
-     * then filter out everything else,
-     * otherwise return complete tree.
-     */
-    if (xvec != NULL){
-	for (i=0; i<xlen; i++)
-	    xml_flag_set(xvec[i], XML_FLAG_MARK);
-    }
-    /* Remove everything that is not marked */
-    if (!xml_flag(xret, XML_FLAG_MARK))
-	if (xml_tree_prune_flagged_sub(xret, XML_FLAG_MARK, 1, NULL) < 0)
 	goto done;
-    /* reset flag */
-    if (xml_apply(xret, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, (void*)XML_FLAG_MARK) < 0)
-	goto done;
-    /* Add default global values */
-    if (xml_global_defaults(h, xret, nsc, xpath, yspec, 0) < 0)
-	goto done;
-    /* Add default recursive values */
-    if (xml_default_recurse(xret, 0) < 0)
-	goto done;
-    /* Pre-NACM access step */
-    xnacm = clicon_nacm_cache(h);
-    if (xnacm != NULL){ /* Do NACM validation */
-	if (xpath_vec(xret, nsc, "%s", &xvecnacm, &xlennacm, xpath?xpath:"/") < 0)
-	    goto done;
-	/* NACM datanode/module read validation */
-	if (nacm_datanode_read(h, xret, xvecnacm, xlennacm, username, xnacm) < 0) 
-	    goto done;
-    }
-    cprintf(cbret, "<rpc-reply xmlns=\"%s\">", NETCONF_BASE_NAMESPACE);     /* OK */
-    if (xret==NULL)
-	cprintf(cbret, "<data/>");
-    else{
-	if (xml_name_set(xret, NETCONF_OUTPUT_DATA) < 0)
-	    goto done;
-	/* Top level is data, so add 1 to depth if significant */
-	if (clicon_xml2cbuf(cbret, xret, 0, 0, depth>0?depth+1:depth) < 0)
-	    goto done;
-    }
-    cprintf(cbret, "</rpc-reply>");
  ok:
     retval = 0;
  done:
@@ -886,12 +888,6 @@ get_common(clicon_handle        h,
 	xml_free(xerr);
     if (xpath)
 	free(xpath);
-    if (xvec)
-	free(xvec);
-    if (xvecnacm)
-	free(xvecnacm);
-    if (xret)
-	xml_free(xret);
     return retval;
 }
 

example/main/example_backend.c

@@ -1092,7 +1092,7 @@ example_daemon(clicon_handle h)
     yang_stmt *yspec;
 
     /* Read state file (or should this be in init/start?) */
-    if (_state && _state_file && _state_file_cached){
+    if (_state && _state_file && _state_file_cached && 0){
 	yspec = clicon_dbspec_yang(h);
 	if ((fp = fopen(_state_file, "r")) == NULL){
 	    clicon_err(OE_UNIX, errno, "open(%s)", _state_file);

include/clixon_custom.h

@@ -124,3 +124,8 @@
  * draft-wwlh-netconf-list-pagination-rc-01
  */
 #define LIST_PAGINATION
+
+/*! Enable "remaining" attribute (sub-feature of list pagination)
+ * As defined in draft-wwlh-netconf-list-pagination-00 using Yang metadata value [RFC7952] 
+ */
+#undef LIST_PAGINATION_REMAINING

test/example_social.sh

@@ -1,7 +1,7 @@
 #!/usr/bin/env bash
 # Example-social from draft-netconf-list-pagination-00.txt appendix A.1
 # Assumes variable fexample is set to name of yang file
-# Note inverted pattern is commented
+# Note audit-logs/audit-log/outcome is changed from mandatory to default
 
 cat <<EOF > $fexample
    module example-social {
@@ -281,7 +281,7 @@ cat <<EOF > $fexample
          }
          leaf outcome {
            type boolean;
-           mandatory true;
+           default true; /* Note changed from mandatory in original */
            description
              "Indicate if request was permitted.";
          }

test/test_paging_state.sh

@@ -7,8 +7,8 @@
 # Magic line must be first in script (see README.md)
 s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi
 
-#echo "...skipped: Must run interactvely"
+echo "...skipped: Must run interactvely"
-#if [ "$s" = $0 ]; then exit 0; else return 0; fi
+if [ "$s" = $0 ]; then exit 0; else return 0; fi
 
 APPNAME=example
 
@@ -16,6 +16,9 @@ cfg=$dir/conf.xml
 fexample=$dir/example-social.yang
 fstate=$dir/mystate.xml
 
+# For 1M test,m use an external file since the generation takes considerable time
+#fstate=~/tmp/mystate.xml
+
 # Common example-module spec (fexample must be set)
 . ./example_social.sh
 
@@ -59,14 +62,10 @@ new "generate state with $perfnr list entries"
 echo "<audit-logs xmlns=\"http://example.com/ns/example-social\">" >> $fstate
 for (( i=0; i<$perfnr; i++ )); do  
     echo "  <audit-log>" >> $fstate
-    mon=$(( ( RANDOM % 10 ) ))
+    echo "    <timestamp>2021-09-05T018:48:11Z</timestamp>" >> $fstate
-    day=$(( ( RANDOM % 10 ) ))
-    hour=$(( ( RANDOM % 10 ) ))
-    echo "    <timestamp>2020-0$mon-0$dayT0$hour:48:11Z</timestamp>" >> $fstate
     echo "    <member-id>bob$i</member-id>" >> $fstate
     echo "    <source-ip>192.168.1.32</source-ip>" >> $fstate
     echo "    <request>POST</request>" >> $fstate
-    echo "    <outcome>true</outcome>" >> $fstate
     echo "  </audit-log>" >> $fstate
 done
 echo -n "</audit-logs>" >> $fstate # No CR
@@ -90,6 +89,7 @@ wait_backend
 
 # XXX How to run without using a terminal?
 new "cli show"
+echo "$clixon_cli -1 -f $cfg -l o show pagination xpath /es:audit-logs/es:audit-log cli"
 $clixon_cli -1 -f $cfg -l o show pagination xpath /es:audit-logs/es:audit-log cli
 
 if [ $BE -ne 0 ]; then

yang/clixon/clixon-config@2021-07-11.yang

@@ -692,7 +692,7 @@ module clixon-config {
 	    type int32;
 	    default 24;
 	    description
-		"Set to number of CLI terminal rows for pageing/scrolling. 0 means unlimited.
+		"Set to number of CLI terminal rows for paging/scrolling. 0 means unlimited.
                  The number is set statically UNLESS:
                  - there is no terminal, such as file input, in which case nr lines is 0
                  - there is a terminal sufficiently powerful to read the number of lines from

yang/clixon/clixon-netconf-list-pagination@2021-08-27.yang

@@ -80,7 +80,7 @@ module clixon-netconf-list-pagination {
           represent any value greater than or equal to 2^32-1
           elements.";
     }
-    grouping pageing-parameters {
+    grouping paging-parameters {
 	leaf list-pagination {
 	    type boolean;
 	    default false;
@@ -173,10 +173,10 @@ module clixon-netconf-list-pagination {
 	}
     }
     augment /nc:get-config/nc:input {
-	uses pageing-parameters;
+	uses paging-parameters;
     }
     // extending the get operation
     augment /nc:get/nc:input {
-	uses pageing-parameters;
+	uses paging-parameters;
     }
 }