diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c28e602..be38bd76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,8 @@ Expected: Early March 2020 * New clixon-config@2020-02-22.yang revision * Search index extension `search_index` for declaring which non-key variables are search indexes * Added `clixon-stats` state for clixon XML and memory statistics. + * Added: CLICON_CLI_BUF_START and CLICON_CLI_BUF_THRESHOLD so you can change the start and + threshold of quadratic and linear growth of CLIgen buffers (cbuf:s) * JSON parse error messages change from ` on line x: syntax error,..` to `json_parse: line x: syntax error` * Unknown-element error message is more descriptive, eg from `namespace is: urn:example:clixon` to: `Failed to find YANG spec of XML node: x with parent: xp in namespace urn:example:clixon`. * C-API parse and validation API more capable @@ -64,6 +66,10 @@ Expected: Early March 2020 ### Minor changes +* Memory footprint + * Do not autopopulate namespace cache, instead use on-demand, see `xml2ns()`. + * Set CBUF start level to 256 (`CLICON_CLI_BUF_START` option) + * Reduced xml child vector default size from 4 to 1 with quadratic growoth to 64K then linear * C-API: * Added instrumentation: `xml_stats` and `xml_stats_global`. * Added object-based `clixon_xvec` as a new programming construct for contiguous XML object vectors. diff --git a/README.md b/README.md index 44fe2550..54b606af 100644 --- a/README.md +++ b/README.md @@ -4,20 +4,19 @@ [![Build Status](https://travis-ci.org/clicon/clixon.png)](https://travis-ci.org/clicon/clixon) [![Documentation Status](https://readthedocs.org/projects/clixon-docs/badge/?version=latest)](https://clixon-docs.readthedocs.io/en/latest/?badge=latest) - Clixon is a YANG-based configuration manager, with interactive CLI, NETCONF and RESTCONF interfaces, an embedded database and transaction mechanism. See [main documentation](https://clixon-docs.readthedocs.io) and [project page](https://www.clicon.org). -Clixon interaction is best done posting issues, pull requests, or joining the -[slack channel](https://clixondev.slack.com). -[Slack invite](https://join.slack.com/t/clixondev/shared_invite/enQtMzI3OTM4MzA3Nzk3LTA3NWM4OWYwYWMxZDhiYTNhNjRkNjQ1NWI1Zjk5M2JjMDk4MTUzMTljYTZiYmNhODkwMDI2ZTkyNWU3ZWMyN2U). - Clixon is open-source and dual licensed. Either Apache License, Version 2.0 or GNU General Public License Version 2; you choose. See [LICENSE.md](LICENSE.md) for the license. +Clixon interaction is best done posting issues, pull requests, or joining the +[slack channel](https://clixondev.slack.com). +[Slack invite](https://join.slack.com/t/clixondev/shared_invite/enQtMzI3OTM4MzA3Nzk3LTA3NWM4OWYwYWMxZDhiYTNhNjRkNjQ1NWI1Zjk5M2JjMDk4MTUzMTljYTZiYmNhODkwMDI2ZTkyNWU3ZWMyN2U). + [Netgate](https://www.netgate.com/) sponsors Clixon. diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index ee69740c..cd3fddd5 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -266,24 +266,23 @@ client_get_streams(clicon_handle h, */ static int clixon_stats_get_db(clicon_handle h, - char *name, + char *dbname, cbuf *cb) { int retval = -1; cxobj *xt = NULL; uint64_t nr = 0; size_t sz = 0; - - if (xmldb_get(h, "running", NULL, NULL, &xt) < 0) - goto done; + db_elmnt *de = NULL; + + /* This is the db cache */ + if ((de = clicon_db_elmnt_get(h, dbname)) != NULL) + xt = de->de_xml; xml_stats(xt, &nr, &sz); cprintf(cb, "%s%" PRIu64 "" "%" PRIu64 "", - name, nr, sz); + dbname, nr, sz); retval = 0; - done: - if (xt) - free(xt); return retval; } diff --git a/apps/backend/backend_main.c b/apps/backend/backend_main.c index f82d4d20..b87338a9 100644 --- a/apps/backend/backend_main.c +++ b/apps/backend/backend_main.c @@ -460,6 +460,8 @@ main(int argc, char *dir; gid_t gid = -1; cvec *nsctx_global = NULL; /* Global namespace context */ + size_t cligen_buflen; + size_t cligen_bufthreshold; /* In the startup, logs to stderr & syslog and debug flag set later */ clicon_log_init(__PROGRAM__, LOG_INFO, logdst); @@ -623,11 +625,16 @@ main(int argc, clicon_argv_set(h, argv0, argc, argv); clicon_log_init(__PROGRAM__, debug?LOG_DEBUG:LOG_INFO, logdst); - + /* Defer: Wait to the last minute to print help message */ if (help) usage(h, argv[0]); + /* Init cligen buffers */ + cligen_buflen = clicon_option_int(h, "CLICON_CLI_BUF_START"); + cligen_bufthreshold = clicon_option_int(h, "CLICON_CLI_BUF_THRESHOLD"); + cbuf_alloc_set(cligen_buflen, cligen_bufthreshold); + #ifndef HAVE_LIBXML2 if (clicon_yang_regexp(h) == REGEXP_LIBXML2){ clicon_err(OE_FATAL, 0, "CLICON_YANG_REGEXP set to libxml2, but HAVE_LIBXML2 not set (Either change CLICON_YANG_REGEXP to posix, or run: configure --with-libxml2))"); diff --git a/apps/cli/cli_main.c b/apps/cli/cli_main.c index 4885ba0c..654d317b 100644 --- a/apps/cli/cli_main.c +++ b/apps/cli/cli_main.c @@ -289,6 +289,8 @@ main(int argc, char **argv) int tabmode; char *dir; cvec *nsctx_global = NULL; /* Global namespace context */ + size_t cligen_buflen; + size_t cligen_bufthreshold; /* Defaults */ once = 0; @@ -449,6 +451,11 @@ main(int argc, char **argv) if (help) usage(h, argv[0]); + /* Init cligen buffers */ + cligen_buflen = clicon_option_int(h, "CLICON_CLI_BUF_START"); + cligen_bufthreshold = clicon_option_int(h, "CLICON_CLI_BUF_THRESHOLD"); + cbuf_alloc_set(cligen_buflen, cligen_bufthreshold); + if (clicon_yang_regexp(h) == REGEXP_LIBXML2){ #ifdef HAVE_LIBXML2 /* Enable XSD libxml2 regex engine */ diff --git a/apps/netconf/netconf_main.c b/apps/netconf/netconf_main.c index c92ac114..52f842c3 100644 --- a/apps/netconf/netconf_main.c +++ b/apps/netconf/netconf_main.c @@ -398,6 +398,8 @@ main(int argc, char *str; uint32_t id; cvec *nsctx_global = NULL; /* Global namespace context */ + size_t cligen_buflen; + size_t cligen_bufthreshold; /* Create handle */ if ((h = clicon_handle_init()) == NULL) @@ -512,6 +514,11 @@ main(int argc, /* Access the remaining argv/argc options (after --) w clicon-argv_get() */ clicon_argv_set(h, argv0, argc, argv); + /* Init cligen buffers */ + cligen_buflen = clicon_option_int(h, "CLICON_CLI_BUF_START"); + cligen_bufthreshold = clicon_option_int(h, "CLICON_CLI_BUF_THRESHOLD"); + cbuf_alloc_set(cligen_buflen, cligen_bufthreshold); + /* Add (hardcoded) netconf features in case ietf-netconf loaded here * Otherwise it is loaded in netconf_module_load below */ diff --git a/apps/restconf/restconf_main.c b/apps/restconf/restconf_main.c index 23e192bd..a932c540 100644 --- a/apps/restconf/restconf_main.c +++ b/apps/restconf/restconf_main.c @@ -588,6 +588,8 @@ main(int argc, clixon_plugin *cp = NULL; uint32_t id = 0; cvec *nsctx_global = NULL; /* Global namespace context */ + size_t cligen_buflen; + size_t cligen_bufthreshold; /* In the startup, logs to stderr & debug flag set later */ clicon_log_init(__PROGRAM__, LOG_INFO, logdst); @@ -694,6 +696,11 @@ main(int argc, /* Access the remaining argv/argc options (after --) w clicon-argv_get() */ clicon_argv_set(h, argv0, argc, argv); + /* Init cligen buffers */ + cligen_buflen = clicon_option_int(h, "CLICON_CLI_BUF_START"); + cligen_bufthreshold = clicon_option_int(h, "CLICON_CLI_BUF_THRESHOLD"); + cbuf_alloc_set(cligen_buflen, cligen_bufthreshold); + /* Add (hardcoded) netconf features in case ietf-netconf loaded here * Otherwise it is loaded in netconf_module_load below */ diff --git a/doc/DEVELOP.md b/doc/DEVELOP.md index bc88a7f7..d615e6ae 100644 --- a/doc/DEVELOP.md +++ b/doc/DEVELOP.md @@ -96,6 +96,8 @@ EOF valgrind --leak-check=full --show-leak-kinds=all clixon_netconf -qf /tmp/myconf.xml -y /tmp/my.yang valgrind --tool=callgrind clixon_netconf -qf /tmp/myconf.xml -y /tmp/my.yang sudo kcachegrind + valgrind --tool=massif clixon_netconf -qf /tmp/myconf.xml -y /tmp/my.yang + massif-visualizer ``` ## New release diff --git a/lib/clixon/clixon_xml.h b/lib/clixon/clixon_xml.h index 25cb68c2..3db03bf6 100644 --- a/lib/clixon/clixon_xml.h +++ b/lib/clixon/clixon_xml.h @@ -161,6 +161,7 @@ int xml_child_insert_pos(cxobj *x, cxobj *xc, int i); int xml_childvec_set(cxobj *x, int len); cxobj **xml_childvec_get(cxobj *x); cxobj *xml_new(char *name, cxobj *xn_parent, yang_stmt *spec); +cxobj *xml_new2(char *name, cxobj *xn_parent, enum cxobj_type type); yang_stmt *xml_spec(cxobj *x); int xml_spec_set(cxobj *x, yang_stmt *spec); cg_var *xml_cv(cxobj *x); diff --git a/lib/src/clixon_xml.c b/lib/src/clixon_xml.c index 17ca920f..8491e16c 100644 --- a/lib/src/clixon_xml.c +++ b/lib/src/clixon_xml.c @@ -78,10 +78,16 @@ /* * Constants */ -/* How many XML children to start with if any (and then add exponentialy) */ -#define XML_CHILDVEC_MAX_DEFAULT 4 -/* Initial length of x_value malloced string */ -#define XML_VALUE_MAX_DEFAULT 32 +/* How many XML children to start with if any. Then add quadratic until threshold when + * add lineraly */ +#define XML_CHILDVEC_SIZE_START 1 +#define XML_CHILDVEC_SIZE_THRESHOLD 65536 + +/* Intention of these macros is to guard against access of type-specific fields + * As debug they can contain an assert. + */ +#define is_element(x) (xml_type(x)==CX_ELMNT) +#define is_bodyattr(x) (xml_type(x)==CX_BODY || xml_type(x)==CX_ATTR) /* * Types @@ -150,28 +156,47 @@ struct search_index{ * vocabulary's local names that is effective in avoiding name clashes. */ struct xml{ + enum cxobj_type x_type; /* type of node: element, attribute, body */ char *x_name; /* name of node */ char *x_prefix; /* namespace localname N, called prefix */ + uint16_t x_flags; /* Flags according to XML_FLAG_* */ struct xml *x_up; /* parent node in hierarchy if any */ - struct xml **x_childvec; /* vector of children nodes (XXX: use clixon_vec ) */ - int x_childvec_len;/* Number of children */ - int x_childvec_max;/* Length of allocated vector */ - enum cxobj_type x_type; /* type of node: element, attribute, body */ - cbuf *x_value_cb; /* attribute and body nodes have values (XXX: this consumes - memory) cv? */ - int x_flags; /* Flags according to XML_FLAG_* */ - yang_stmt *x_spec; /* Pointer to specification, eg yang, by - reference, dont free */ - cg_var *x_cv; /* Cached value as cligen variable (eg xml_cmp) */ - cvec *x_ns_cache; /* Cached vector of namespaces */ int _x_vector_i; /* internal use: xml_child_each */ int _x_i; /* internal use for sorting: see xml_enumerate and xml_cmp */ + /*----- next is body/attribute only */ + cbuf *x_value_cb; /* attribute and body nodes have values (XXX: this consumes + memory) cv? */ + /*----- up to here is common to all next is element only */ + struct xml **x_childvec; /* vector of children nodes (XXX: use clixon_vec ) */ + int x_childvec_len;/* Number of children */ + int x_childvec_max;/* Length of allocated vector */ + + + cvec *x_ns_cache; /* Cached vector of namespaces */ + yang_stmt *x_spec; /* Pointer to specification, eg yang, + by reference, dont free */ + cg_var *x_cv; /* Cached value as cligen variable (eg xml_cmp) */ #ifdef XML_EXPLICIT_INDEX struct search_index *x_search_index; /* explicit search index vectors */ #endif }; +/* This is experimental variant of struct xml for use by non-elements to save space + */ +struct xmlbody{ + enum cxobj_type xb_type; /* type of node: element, attribute, body */ + char *xb_name; /* name of node */ + char *xb_prefix; /* namespace localname N, called prefix */ + uint16_t xb_flags; /* Flags according to XML_FLAG_* */ + struct xml *xb_up; /* parent node in hierarchy if any */ + int _xb_vector_i; /* internal use: xml_child_each */ + int _xb_i; /* internal use for sorting: + see xml_enumerate and xml_cmp */ + cbuf *xb_value_cb; /* attribute and body nodes have values (XXX: this consumes + memory) cv? */ +}; + /* * Variables */ @@ -225,23 +250,33 @@ xml_stats_one(cxobj *x, sz += strlen(x->x_name) + 1; if (x->x_prefix) sz += strlen(x->x_prefix) + 1; - sz += x->x_childvec_max*sizeof(struct xml*); - if (x->x_value_cb) - sz += cbuf_buflen(x->x_value_cb); - if (x->x_cv) - sz += cv_size(x->x_cv); - if (x->x_ns_cache) - sz += cvec_size(x->x_ns_cache); + switch (xml_type(x)){ + case CX_ELMNT: + sz += x->x_childvec_max*sizeof(struct xml*); + if (x->x_ns_cache) + sz += cvec_size(x->x_ns_cache); + if (x->x_cv) + sz += cv_size(x->x_cv); #ifdef XML_EXPLICIT_INDEX - if (x->x_search_index){ - /* XXX: only one */ - sz += sizeof(struct search_index); - if (x->x_search_index->si_name) - sz += strlen(x->x_search_index->si_name)+1; - if (x->x_search_index->si_xvec) - sz += clixon_xvec_len(x->x_search_index->si_xvec)*sizeof(struct cxobj*); - } + if (x->x_search_index){ + /* XXX: only one */ + sz += sizeof(struct search_index); + if (x->x_search_index->si_name) + sz += strlen(x->x_search_index->si_name)+1; + if (x->x_search_index->si_xvec) + sz += clixon_xvec_len(x->x_search_index->si_xvec)*sizeof(struct cxobj*); + } #endif + break; + case CX_BODY: + case CX_ATTR: + if (x->x_value_cb) + sz += cbuf_buflen(x->x_value_cb); + + break; + default: + break; + } if (szp) *szp = sz; clicon_debug(1, "%s %" PRIu64, __FUNCTION__, sz); @@ -356,6 +391,8 @@ char* nscache_get(cxobj *x, char *prefix) { + if (!is_element(x)) + return NULL; if (x->x_ns_cache != NULL) return xml_nsctx_get(x->x_ns_cache, prefix); return NULL; @@ -373,6 +410,8 @@ nscache_get_prefix(cxobj *x, char *namespace, char **prefix) { + if (!is_element(x)) + return 0; if (x->x_ns_cache != NULL) return xml_nsctx_get_prefix(x->x_ns_cache, namespace, prefix); return 0; @@ -387,6 +426,8 @@ nscache_get_prefix(cxobj *x, cvec * nscache_get_all(cxobj *x) { + if (!is_element(x)) + return NULL; return x->x_ns_cache; } @@ -405,6 +446,8 @@ nscache_set(cxobj *x, { int retval = -1; + if (!is_element(x)) + return 0; if (x->x_ns_cache == NULL){ if ((x->x_ns_cache = xml_nsctx_init(prefix, namespace)) == NULL) goto done; @@ -429,6 +472,8 @@ nscache_replace(cxobj *x, { int retval = -1; + if (!is_element(x)) + return 0; if (x->x_ns_cache != NULL){ xml_nsctx_free(x->x_ns_cache); x->x_ns_cache = NULL; @@ -449,6 +494,9 @@ nscache_replace(cxobj *x, int nscache_clear(cxobj *x) { + + if (!is_element(x)) + return 0; if (x->x_ns_cache != NULL){ xml_nsctx_free(x->x_ns_cache); x->x_ns_cache = NULL; @@ -521,6 +569,8 @@ xml_flag_reset(cxobj *xn, char* xml_value(cxobj *xn) { + if (!is_bodyattr(xn)) + return NULL; return xn->x_value_cb?cbuf_get(xn->x_value_cb):NULL; } @@ -534,10 +584,18 @@ int xml_value_set(cxobj *xn, char *val) { - int retval = -1; - + int retval = -1; + size_t sz; + + if (!is_bodyattr(xn)) + return 0; + if (val == NULL){ + clicon_err(OE_XML, EINVAL, "value is NULL"); + goto done; + } + sz = strlen(val)+1; if (xn->x_value_cb == NULL){ - if ((xn->x_value_cb = cbuf_new()) == NULL){ + if ((xn->x_value_cb = cbuf_new_alloc(sz)) == NULL){ clicon_err(OE_XML, errno, "cbuf_new"); goto done; } @@ -560,10 +618,18 @@ int xml_value_append(cxobj *xn, char *val) { - int retval = -1; - + int retval = -1; + size_t sz; + + if (!is_bodyattr(xn)) + return 0; + if (val == NULL){ + clicon_err(OE_XML, EINVAL, "value is NULL"); + goto done; + } + sz = strlen(val)+1; if (xn->x_value_cb == NULL){ - if ((xn->x_value_cb = cbuf_new()) == NULL){ + if ((xn->x_value_cb = cbuf_new_alloc(sz)) == NULL){ clicon_err(OE_XML, errno, "cbuf_new"); goto done; } @@ -611,6 +677,8 @@ xml_type_set(cxobj *xn, int xml_child_nr(cxobj *xn) { + if (!is_element(xn)) + return 0; return xn->x_childvec_len; } @@ -628,6 +696,8 @@ xml_child_nr_notype(cxobj *xn, cxobj *x = NULL; int nr = 0; + if (!is_element(xn)) + return 0; while ((x = xml_child_each(xn, x, -1)) != NULL) { if (xml_type(x) != type) nr++; @@ -649,6 +719,8 @@ xml_child_nr_type(cxobj *xn, cxobj *x = NULL; int len = 0; + if (!is_element(xn)) + return 0; while ((x = xml_child_each(xn, x, type)) != NULL) len++; return len; @@ -666,6 +738,8 @@ cxobj * xml_child_i(cxobj *xn, int i) { + if (!is_element(xn)) + return NULL; if (i < xn->x_childvec_len) return xn->x_childvec[i]; return NULL; @@ -686,6 +760,8 @@ xml_child_i_type(cxobj *xn, cxobj *x = NULL; int it = 0; + if (!is_element(xn)) + return NULL; while ((x = xml_child_each(xn, x, type)) != NULL) { if (x->x_type == type && (i == it++)) return x; @@ -704,6 +780,8 @@ xml_child_i_set(cxobj *xt, int i, cxobj *xc) { + if (!is_element(xt)) + return NULL; if (i < xt->x_childvec_len) xt->x_childvec[i] = xc; return 0; @@ -724,6 +802,8 @@ xml_child_order(cxobj *xp, cxobj *x = NULL; int i = 0; + if (!is_element(xp)) + return -1; while ((x = xml_child_each(xp, x, -1)) != NULL) { if (x == xc) return i; @@ -758,6 +838,8 @@ xml_child_each(cxobj *xparent, if (xparent == NULL) return NULL; + if (!is_element(xparent)) + return NULL; for (i=xprev?xprev->_x_vector_i+1:0; ix_childvec_len; i++){ xn = xparent->x_childvec[i]; if (xn == NULL) @@ -780,9 +862,14 @@ static int xml_child_append(cxobj *x, cxobj *xc) { + if (!is_element(x)) + return 0; x->x_childvec_len++; if (x->x_childvec_len > x->x_childvec_max){ - x->x_childvec_max = x->x_childvec_max?2*x->x_childvec_max:XML_CHILDVEC_MAX_DEFAULT; + if (x->x_childvec_len < XML_CHILDVEC_SIZE_THRESHOLD) + x->x_childvec_max = x->x_childvec_max?2*x->x_childvec_max:XML_CHILDVEC_SIZE_START; + else + x->x_childvec_max += XML_CHILDVEC_SIZE_THRESHOLD; x->x_childvec = realloc(x->x_childvec, x->x_childvec_max*sizeof(cxobj*)); if (x->x_childvec == NULL){ clicon_err(OE_XML, errno, "realloc"); @@ -805,6 +892,8 @@ xml_child_insert_pos(cxobj *xp, { size_t size; + if (!is_element(xp)) + return 0; xp->x_childvec_len++; if (xp->x_childvec_len > xp->x_childvec_max){ xp->x_childvec_max = xp->x_childvec_max?2*xp->x_childvec_max:XML_CHILDVEC_MAX_DEFAULT; @@ -831,6 +920,8 @@ int xml_childvec_set(cxobj *x, int len) { + if (!is_element(x)) + return 0; x->x_childvec_len = len; x->x_childvec_max = len; if (x->x_childvec) @@ -847,6 +938,8 @@ xml_childvec_set(cxobj *x, cxobj ** xml_childvec_get(cxobj *x) { + if (!is_element(x)) + return NULL; return x->x_childvec; } @@ -895,12 +988,55 @@ xml_new(char *name, return x; } +/*! Create new xml node given a name and parent. Free with xml_free(). + */ +cxobj * +xml_new2(char *name, + cxobj *xp, + enum cxobj_type type) +{ + struct xml *x = NULL; + size_t sz; + + switch (type){ + case CX_ELMNT: + sz = sizeof(struct xml); + break; + case CX_ATTR: + case CX_BODY: + sz = sizeof(struct xmlbody); + break; + default: + clicon_err(OE_XML, EINVAL, "Invalid type: %d", type); + return NULL; + break; + } + if ((x = malloc(sz)) == NULL){ + clicon_err(OE_XML, errno, "malloc"); + return NULL; + } + memset(x, 0, sz); + xml_type_set(x, type); + if (name && (xml_name_set(x, name)) < 0) + return NULL; + if (xp){ + xml_parent_set(x, xp); + if (xml_child_append(xp, x) < 0) + return NULL; + x->_x_i = xml_child_nr(xp)-1; + } + _stats_nr++; + return x; +} + /*! Return yang spec of node. * Not necessarily set. Either has not been set yet (by xml_spec_set( or anyxml. */ yang_stmt * xml_spec(cxobj *x) { + if (!is_element(x)) + return NULL; return x->x_spec; } @@ -908,6 +1044,8 @@ int xml_spec_set(cxobj *x, yang_stmt *spec) { + if (!is_element(x)) + return 0; x->x_spec = spec; return 0; } @@ -921,6 +1059,8 @@ xml_spec_set(cxobj *x, cg_var * xml_cv(cxobj *x) { + if (!is_element(x)) + return NULL; return x->x_cv; } @@ -934,6 +1074,8 @@ int xml_cv_set(cxobj *x, cg_var *cv) { + if (!is_element(x)) + return 0; if (x->x_cv) cv_free(x->x_cv); x->x_cv = cv; @@ -958,12 +1100,14 @@ xml_cv_set(cxobj *x, * @see xml_find_type A more generic function fixes (1) and (2) above */ cxobj * -xml_find(cxobj *x_up, +xml_find(cxobj *xp, char *name) { cxobj *x = NULL; - while ((x = xml_child_each(x_up, x, -1)) != NULL) + if (!is_element(xp)) + return NULL; + while ((x = xml_child_each(xp, x, -1)) != NULL) if (strcmp(name, xml_name(x)) == 0) break; /* x is set */ return x; @@ -1011,6 +1155,7 @@ xml_addsub(cxobj *xp, goto done; /* 2. Get child default namespace */ if (pns && + xml_type(xc) == CX_ELMNT && (xa = xml_find_type(xc, NULL, "xmlns", CX_ATTR)) != NULL && (cns = xml_value(xa)) != NULL){ /* 3. check if same, if so remove child's */ @@ -1044,6 +1189,8 @@ xml_wrap_all(cxobj *xp, { cxobj *xw; /* new wrap node */ + if (!is_element(xp)) + return NULL; if ((xw = xml_new(tag, NULL, NULL)) == NULL) goto done; while (xp->x_childvec_len) @@ -1131,19 +1278,24 @@ xml_child_rm(cxobj *xp, int retval = -1; cxobj *xc = NULL; + if (!is_element(xp)) + return 0; if ((xc = xml_child_i(xp, i)) == NULL){ clicon_err(OE_XML, 0, "Child not found"); goto done; } -#ifdef XML_EXPLICIT_INDEX - if (xml_search_index_p(xc)) - xml_search_child_rm(xp, xc); -#endif - xp->x_childvec[i] = NULL; xml_parent_set(xc, NULL); + xp->x_childvec[i] = NULL; xp->x_childvec_len--; if (ix_childvec_len) memmove(&xp->x_childvec[i], &xp->x_childvec[i+1], (xp->x_childvec_len-i)*sizeof(cxobj*)); +#ifdef XML_EXPLICIT_INDEX + if (xml_type(xc) == CX_ELMNT){ + if (xml_search_index_p(xc)) + xml_search_child_rm(xp, xc); + + } +#endif retval = 0; done: return retval; @@ -1190,20 +1342,22 @@ xml_rm(cxobj *xc) * @retval -1 Error */ int -xml_rm_children(cxobj *x, +xml_rm_children(cxobj *xp, enum cxobj_type type) { int retval = -1; cxobj *xc; int i; - for (i=0; i_x_i = i++; return 0; @@ -1327,6 +1487,8 @@ xml_enumerate_reset(cxobj *xp) { cxobj *x = NULL; + if (!is_element(xp)) + return 0; while ((x = xml_child_each(xp, x, -1)) != NULL) x->_x_i = 0; return 0; @@ -1361,6 +1523,8 @@ xml_body(cxobj *xn) { cxobj *xb = NULL; + if (!is_element(xn)) + return NULL; while ((xb = xml_child_each(xn, xb, CX_BODY)) != NULL) return xml_value(xb); return NULL; @@ -1377,6 +1541,8 @@ xml_body_get(cxobj *xt) { cxobj *xb = NULL; + if (!is_element(xt)) + return NULL; while ((xb = xml_child_each(xt, xb, CX_BODY)) != NULL) return xb; return NULL; @@ -1405,6 +1571,8 @@ xml_find_type_value(cxobj *xt, { cxobj *x; + if (!is_element(xt)) + return NULL; if ((x = xml_find_type(xt, prefix, name, type)) != NULL) return xml_value(x); return NULL; @@ -1435,6 +1603,8 @@ xml_find_type(cxobj *xt, int pmatch; /* prefix match */ char *xprefix; /* xprefix */ + if (!is_element(xt)) + return NULL; while ((x = xml_child_each(xt, x, type)) != NULL) { if (prefix){ xprefix = xml_prefix(x); @@ -1469,6 +1639,8 @@ xml_find_value(cxobj *xt, { cxobj *x = NULL; + if (!is_element(xt)) + return NULL; while ((x = xml_child_each(xt, x, -1)) != NULL) if (strcmp(name, xml_name(x)) == 0) return xml_value(x); @@ -1492,6 +1664,8 @@ xml_find_body(cxobj *xt, { cxobj *x=NULL; + if (!is_element(xt)) + return NULL; while ((x = xml_child_each(xt, x, -1)) != NULL) if (strcmp(name, xml_name(x)) == 0) return xml_body(x); @@ -1520,6 +1694,8 @@ xml_find_body_obj(cxobj *xt, cxobj *x = NULL; char *bstr; + if (!is_element(xt)) + return NULL; while ((x = xml_child_each(xt, x, CX_ELMNT)) != NULL) { if (strcmp(name, xml_name(x))) continue; @@ -1545,23 +1721,32 @@ xml_free(cxobj *x) free(x->x_name); if (x->x_prefix) free(x->x_prefix); - if (x->x_value_cb) - cbuf_free(x->x_value_cb); - for (i=0; ix_childvec_len; i++){ - if ((xc = x->x_childvec[i]) != NULL){ - xml_free(xc); - x->x_childvec[i] = NULL; + switch (xml_type(x)){ + case CX_ELMNT: + for (i=0; ix_childvec_len; i++){ + if ((xc = x->x_childvec[i]) != NULL){ + xml_free(xc); + x->x_childvec[i] = NULL; + } } - } - if (x->x_childvec) - free(x->x_childvec); - if (x->x_cv) - cv_free(x->x_cv); - if (x->x_ns_cache) - xml_nsctx_free(x->x_ns_cache); + if (x->x_childvec) + free(x->x_childvec); + if (x->x_cv) + cv_free(x->x_cv); + if (x->x_ns_cache) + xml_nsctx_free(x->x_ns_cache); #ifdef XML_EXPLICIT_INDEX - xml_search_index_free(x); + xml_search_index_free(x); #endif + break; + case CX_BODY: + case CX_ATTR: + if (x->x_value_cb) + cbuf_free(x->x_value_cb); + break; + default: + break; + } free(x); _stats_nr--; return 0; @@ -1581,17 +1766,27 @@ xml_copy_one(cxobj *x0, char *s; xml_type_set(x1, xml_type(x0)); - if ((s = xml_value(x0))){ /* malloced string */ - if (xml_value_set(x1, s) < 0) - goto done; - } if ((s = xml_name(x0))) /* malloced string */ if ((xml_name_set(x1, s)) < 0) goto done; if ((s = xml_prefix(x0))) /* malloced string */ if ((xml_prefix_set(x1, s)) < 0) goto done; - xml_spec_set(x1, xml_spec(x0)); + switch (xml_type(x0)){ + case CX_ELMNT: + xml_spec_set(x1, xml_spec(x0)); + break; + case CX_BODY: + case CX_ATTR: + if ((s = xml_value(x0))){ /* malloced string */ + if (xml_value_set(x1, s) < 0) + goto done; + + } + break; + default: + break; + } retval = 0; done: return retval; @@ -1788,6 +1983,8 @@ xml_apply(cxobj *xn, cxobj *x; int ret; + if (!is_element(xn)) + return 0; x = NULL; while ((x = xml_child_each(xn, x, type)) != NULL) { if ((ret = fn(x, arg)) < 0) diff --git a/lib/src/clixon_xml_nsctx.c b/lib/src/clixon_xml_nsctx.c index d84d9430..47081e03 100644 --- a/lib/src/clixon_xml_nsctx.c +++ b/lib/src/clixon_xml_nsctx.c @@ -458,10 +458,12 @@ xml2ns(cxobj *x, } #endif } +#if 0 /* Dont auto-populate all caches, eg startup doesnt need a cache */ /* Set default namespace cache (since code is at this point, * no cache was found */ if (ns && nscache_set(x, prefix, ns) < 0) goto done; +#endif ok: if (namespace) *namespace = ns; diff --git a/lib/src/clixon_xml_sort.c b/lib/src/clixon_xml_sort.c index 81c305be..95f5a0fc 100644 --- a/lib/src/clixon_xml_sort.c +++ b/lib/src/clixon_xml_sort.c @@ -217,6 +217,7 @@ xml_cmp(cxobj *x1, goto done; } } + /* Here x1 and x2 are same type */ y1 = xml_spec(x1); y2 = xml_spec(x2); if (same){ @@ -1034,13 +1035,15 @@ xml_sort_verify(cxobj *x0, retval = 1; goto done; } - xml_enumerate_children(x0); - while ((x = xml_child_each(x0, x, -1)) != NULL) { - if (xprev != NULL){ /* Check xprev <= x */ - if (xml_cmp(xprev, x, 1, 0, NULL) > 0) - goto done; + if (xml_type(x0) == CX_ELMNT){ + xml_enumerate_children(x0); + while ((x = xml_child_each(x0, x, -1)) != NULL) { + if (xprev != NULL){ /* Check xprev <= x */ + if (xml_cmp(xprev, x, 1, 0, NULL) > 0) + goto done; + } + xprev = x; } - xprev = x; } retval = 0; done: diff --git a/lib/src/clixon_xml_vec.c b/lib/src/clixon_xml_vec.c index 42e5b456..e860c30f 100644 --- a/lib/src/clixon_xml_vec.c +++ b/lib/src/clixon_xml_vec.c @@ -97,9 +97,9 @@ clixon_xvec_inc(clixon_xvec *xv) if (xv->xv_max < XVEC_MAX_DEFAULT) xv->xv_max = XVEC_MAX_DEFAULT; else if (xv->xv_max < XVEC_MAX_THRESHOLD) - xv->xv_max *= 2; + xv->xv_max *= 2; /* Double the space - exponential */ else - xv->xv_max += XVEC_MAX_THRESHOLD; + xv->xv_max += XVEC_MAX_THRESHOLD; /* Add - linear growth */ if ((xv->xv_vec = realloc(xv->xv_vec, sizeof(cxobj *) * xv->xv_max)) == NULL){ clicon_err(OE_XML, errno, "realloc"); goto done; diff --git a/test/test_nacm_module_read.sh b/test/test_nacm_module_read.sh index 597ec04c..4fde5025 100755 --- a/test/test_nacm_module_read.sh +++ b/test/test_nacm_module_read.sh @@ -139,14 +139,16 @@ fi new "waiting" wait_backend -new "kill old restconf daemon" -sudo pkill -u $wwwuser -f clixon_restconf +if [ $RC -ne 0 ]; then + new "kill old restconf daemon" + sudo pkill -u $wwwuser -f clixon_restconf -new "start restconf daemon (-a is enable basic authentication)" -start_restconf -f $cfg -- -a + new "start restconf daemon (-a is enable basic authentication)" + start_restconf -f $cfg -- -a -new "waiting" -wait_restconf + new "waiting" + wait_restconf +fi new "auth set authentication config" expecteof "$clixon_netconf -qf $cfg" 0 "$RULES]]>]]>" "^]]>]]>$" @@ -271,9 +273,10 @@ new "guest read state fail" expecteq "$(curl -u guest:bar -sS -X GET http://localhost/restconf/data/clixon-example:state)" 0 '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"access-denied","error-severity":"error","error-message":"default deny"}}} ' -new "Kill restconf daemon" -stop_restconf - +if [ $RC -ne 0 ]; then + new "Kill restconf daemon" + stop_restconf +fi if [ $BE -eq 0 ]; then exit # BE diff --git a/test/test_nacm_module_write.sh b/test/test_nacm_module_write.sh index 67082c42..0bb54111 100755 --- a/test/test_nacm_module_write.sh +++ b/test/test_nacm_module_write.sh @@ -148,14 +148,16 @@ fi new "waiting" wait_backend -new "kill old restconf daemon" -sudo pkill -u $wwwuser -f clixon_restconf +if [ $RC -ne 0 ]; then + new "kill old restconf daemon" + sudo pkill -u $wwwuser -f clixon_restconf -new "start restconf daemon (-a is enable basic authentication)" -start_restconf -f $cfg -- -a + new "start restconf daemon (-a is enable basic authentication)" + start_restconf -f $cfg -- -a -new "waiting" -wait_restconf + new "waiting" + wait_restconf +fi # Set nacm from scratch nacm(){ @@ -252,8 +254,10 @@ expecteq "$(curl -u wilma:bar -sS -X PUT -H "Content-Type: application/yang-data new "default delete list deny" expecteq "$(curl -u wilma:bar -sS -X DELETE http://localhost/restconf/data/clixon-example:translate=key42)" 0 '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"access-denied","error-severity":"error","error-message":"default deny"}}} ' -new "Kill restconf daemon" -stop_restconf +if [ $RC -ne 0 ]; then + new "Kill restconf daemon" + stop_restconf +fi if [ $BE -eq 0 ]; then exit # BE diff --git a/test/test_perf_mem.sh b/test/test_perf_mem.sh index 8b1cbdde..896aa229 100755 --- a/test/test_perf_mem.sh +++ b/test/test_perf_mem.sh @@ -5,6 +5,9 @@ # Magic line must be first in script (see README.md) s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi +# ENable this for massif memory profiling +#clixon_backend="valgrind --tool=massif clixon_backend" + clixon_util_xpath=clixon_util_xpath # Number of list/leaf-list entries in file @@ -58,18 +61,23 @@ cat < $cfg EOF +# Test function +# Arguments: +# 1: nr size of large list testrun(){ nr=$1 - new "generate config with $nr list entries" - echo -n "" > $dir/startup_db - for (( i=0; i<$nr; i++ )); do - echo -n "$i$i" >> $dir/startup_db - done - echo "" >> $dir/startup_db - new "test params: -f $cfg" if [ $BE -ne 0 ]; then + + new "generate config with $nr list entries" + echo -n "" > $dir/startup_db + for (( i=0; i<$nr; i++ )); do + echo -n "$i$i" >> $dir/startup_db + done + echo "" >> $dir/startup_db + + new "kill old backend" sudo clixon_backend -zf $cfg if [ $? -ne 0 ]; then @@ -86,17 +94,19 @@ testrun(){ new "netconf get state" res=$(echo "]]>]]>" | $clixon_netconf -qf $cfg) + echo "Total" echo -n " objects: " echo $res | $clixon_util_xpath -p "/rpc-reply/data/clixon-stats/global/xmlnr" | awk -F ">" '{print $2}' | awk -F "<" '{print $1}' - echo -n " mem: " - # This ony works on Linux - cat /proc/$pid/statm|awk '{print $1*4/1000 "M"}' + if [ -f /proc/$pid/statm ]; then # This ony works on Linux +# cat /proc/$pid/statm + echo -n " mem: " + cat /proc/$pid/statm|awk '{print $1*4/1000 "M"}' + fi for db in running candidate startup; do echo "$db" resdb=$(echo "$res" | $clixon_util_xpath -p "/rpc-reply/data/clixon-stats/datastore[name=\"$db\"]") resdb=${resdb#"nodeset:0:"} -# echo "resdb:$resdb" echo -n " objects: " echo $resdb | $clixon_util_xpath -p "datastore/nr" | awk -F ">" '{print $2}' | awk -F "<" '{print $1}' echo -n " mem: " @@ -111,6 +121,8 @@ testrun(){ err "backend already dead" fi # kill backend + + new "Zap backend" stop_backend -f $cfg fi } diff --git a/yang/clixon/clixon-config@2020-02-22.yang b/yang/clixon/clixon-config@2020-02-22.yang index 385c4f16..49a41d60 100644 --- a/yang/clixon/clixon-config@2020-02-22.yang +++ b/yang/clixon/clixon-config@2020-02-22.yang @@ -43,7 +43,9 @@ module clixon-config { revision 2020-02-22 { description "Added: search index extension, - Added: clixon-stats state for clixon XML and memory statistics."; + Added: clixon-stats state for clixon XML and memory statistics. + Added: CLICON_CLI_BUF_START and CLICON_CLI_BUF_THRESHOLD for quadratic and linear + growth of CLIgen buffers (cbuf:s)"; } revision 2019-09-11 { description @@ -442,14 +444,25 @@ module clixon-config { "Name of CLI history file. If not given, history is not saved. The number of lines is saved is given by CLICON_CLI_HIST_SIZE."; } - leaf CLICON_CLI_HIST_SIZE { - type int32; - default 300; + leaf CLICON_CLI_BUF_START { + type uint32; + default 256; description - "Number of lines to save in CLI history. - Also, if CLICON_CLI_HIST_FILE is set, also the size in lines - of the saved history."; + "CLIgen buffer (cbuf) initial size. + When the buffer needs to grow, the allocation grows quadratic up to a threshold + after which linear growth continues. + See CLICON_CLI_BUF_THRESHOLD"; } + leaf CLICON_CLI_BUF_THRESHOLD { + type uint32; + default 65536; + description + "CLIgen buffer (cbuf) threshold size. + When the buffer exceeds the threshold, the allocation grows by adding the threshold + value to the buffer length. + If 0, the growth continues with quadratic growth. + See CLICON_CLI_BUF_THRESHOLD"; + } leaf CLICON_SOCK_FAMILY { type string; default "UNIX";