diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b98429f..64a5c564 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * Main example yang changed to incorporate augmented state, new revision is 2019-11-15. ### Corrected Bugs +* [xml_parse_string() is slow for a long XML string #96](https://github.com/clicon/clixon/issues/96) * Mandatory variables can no longer be deleted. * [Add missing includes](https://github.com/clicon/clixon/pulls) diff --git a/lib/src/clixon_netconf_lib.c b/lib/src/clixon_netconf_lib.c index 78d18692..69ffb640 100644 --- a/lib/src/clixon_netconf_lib.c +++ b/lib/src/clixon_netconf_lib.c @@ -1369,7 +1369,6 @@ netconf_err2cb(cxobj *xerr, if ((x=xpath_first(xerr, "error-info"))!=NULL) clicon_xml2cbuf(cberr, xml_child_i(x,0), 0, 0, -1); retval = 0; - done: return retval; } diff --git a/lib/src/clixon_options.c b/lib/src/clixon_options.c index 42b8326f..72feb443 100644 --- a/lib/src/clixon_options.c +++ b/lib/src/clixon_options.c @@ -256,7 +256,11 @@ parse_configfile(clicon_handle h, if ((ret = xml_yang_validate_add(h, xc, &xret)) < 0) goto done; if (ret == 0){ - if (netconf_err2cb(xret, &cbret) < 0) + if ((cbret = cbuf_new()) ==NULL){ + clicon_err(OE_XML, errno, "cbuf_new"); + goto done; + } + if (netconf_err2cb(xret, cbret) < 0) goto done; clicon_err(OE_CFG, 0, "Config file validation: %s", cbuf_get(cbret)); goto done; diff --git a/lib/src/clixon_xml.c b/lib/src/clixon_xml.c index 947647e7..be8a36b8 100644 --- a/lib/src/clixon_xml.c +++ b/lib/src/clixon_xml.c @@ -78,6 +78,8 @@ #define XML_TOP_SYMBOL "top" /* 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 /* * Types @@ -125,6 +127,8 @@ struct xml{ int x_childvec_max;/* Length of allocated vector */ enum cxobj_type x_type; /* type of node: element, attribute, body */ char *x_value; /* attribute and body nodes have values */ + uint32_t x_value_len; /* Length of x_value string (excluding null) */ + uint32_t x_value_max; /* allocated size for x_value */ int x_flags; /* Flags according to XML_FLAG_* */ yang_stmt *x_spec; /* Pointer to specification, eg yang, by reference, dont free */ @@ -623,6 +627,42 @@ xml_value(cxobj *xn) return xn->x_value; } +/*! xml value string reallocator + */ +static int +xml_value_realloc(cxobj *xn, + int len0, + char *val) +{ + int retval = -1; + int len; + int diff; + + if (val==NULL) + goto ok; + len = len0 + strlen(val); + diff = xn->x_value_max - (len + 1); + if (diff <= 0){ + while (diff <= 0){ + if (xn->x_value_max == 0) + xn->x_value_max = XML_VALUE_MAX_DEFAULT; + else + xn->x_value_max *= 2; + diff = xn->x_value_max - (len + 1); + } + if ((xn->x_value = realloc(xn->x_value, xn->x_value_max)) == NULL){ + clicon_err(OE_XML, errno, "realloc"); + goto done; + } + } + strncpy(xn->x_value + len0, val, len-len0+1); + xn->x_value_len = len; + ok: + retval = 0; + done: + return retval; +} + /*! Set value of xml node, value is copied * @param[in] xn xml node * @param[in] val new value, null-terminated string, copied by function @@ -633,17 +673,17 @@ int xml_value_set(cxobj *xn, char *val) { - if (xn->x_value){ - free(xn->x_value); - xn->x_value = NULL; + int retval = -1; + + if (xn->x_value && strlen(xn->x_value)){ + xn->x_value[0] = '\0'; + xn->x_value_len = 0; } - if (val){ - if ((xn->x_value = strdup(val)) == NULL){ - clicon_err(OE_XML, errno, "strdup"); - return -1; - } - } - return 0; + if (xml_value_realloc(xn, 0, val) < 0) + goto done; + retval = 0; + done: + return retval; } /*! Append value of xnode, value is copied @@ -656,18 +696,8 @@ char * xml_value_append(cxobj *xn, char *val) { - int len0; - int len; - - len0 = xn->x_value?strlen(xn->x_value):0; - if (val){ - len = len0 + strlen(val); - if ((xn->x_value = realloc(xn->x_value, len+1)) == NULL){ - clicon_err(OE_XML, errno, "realloc"); - return NULL; - } - strncpy(xn->x_value + len0, val, len-len0+1); - } + if (xml_value_realloc(xn, xn->x_value_len, val) < 0) + return NULL; return xn->x_value; } @@ -2141,22 +2171,23 @@ int xml_copy_one(cxobj *x0, cxobj *x1) { + int retval = -1; char *s; xml_type_set(x1, xml_type(x0)); if ((s = xml_value(x0))){ /* malloced string */ - if ((x1->x_value = strdup(s)) == NULL){ - clicon_err(OE_XML, errno, "strdup"); - return -1; - } + if (xml_value_set(x1, s) < 0) + goto done; } if ((s = xml_name(x0))) /* malloced string */ if ((xml_name_set(x1, s)) < 0) - return -1; + goto done; if ((s = xml_prefix(x0))) /* malloced string */ if ((xml_prefix_set(x1, s)) < 0) - return -1; - return 0; + goto done; + retval = 0; + done: + return retval; } /*! Copy xml tree x0 to other existing tree x1 diff --git a/lib/src/clixon_xml_changelog.c b/lib/src/clixon_xml_changelog.c index 80cbb0fc..3335beb2 100644 --- a/lib/src/clixon_xml_changelog.c +++ b/lib/src/clixon_xml_changelog.c @@ -451,7 +451,11 @@ clixon_xml_changelog_init(clicon_handle h) if (ret==1 && (ret = xml_yang_validate_add(h, xt, &xret)) < 0) goto done; if (ret == 0){ /* validation failed */ - if (netconf_err2cb(xret, &cbret) < 0) + if ((cbret = cbuf_new()) ==NULL){ + clicon_err(OE_XML, errno, "cbuf_new"); + goto done; + } + if (netconf_err2cb(xret, cbret) < 0) goto done; clicon_err(OE_YANG, 0, "validation failed: %s", cbuf_get(cbret)); goto done; diff --git a/lib/src/clixon_xml_parse.l b/lib/src/clixon_xml_parse.l index 8d1826c2..f57fb896 100644 --- a/lib/src/clixon_xml_parse.l +++ b/lib/src/clixon_xml_parse.l @@ -94,6 +94,7 @@ ncname {namestart}{namechar}* %s STATEA %s AMPERSAND %s CDATA +%s CDATAEND %s CMNT %s STR %s TEXTDECL @@ -145,9 +146,13 @@ ncname {namestart}{namechar}* "apos;" { BEGIN(_YA->ya_lex_state); clixon_xml_parselval.string = "'"; return CHARDATA;} "quot;" { BEGIN(_YA->ya_lex_state); clixon_xml_parselval.string = "\""; return CHARDATA;} -. { clixon_xml_parselval.string = yytext; return CHARDATA;} \n { clixon_xml_parselval.string = yytext;_YA->ya_linenum++; return (CHARDATA);} -"]]>" { BEGIN(_YA->ya_lex_state); clixon_xml_parselval.string = yytext; return CHARDATA;} +\] { BEGIN(CDATAEND);clixon_xml_parselval.string = yytext;return (CHARDATA);} +[^]\n]+ { clixon_xml_parselval.string = yytext; return CHARDATA;} + +\n { clixon_xml_parselval.string = yytext;_YA->ya_linenum++; return (CHARDATA);} +"]>" { BEGIN(_YA->ya_lex_state); clixon_xml_parselval.string = yytext; return CHARDATA;} +. { BEGIN(CDATA); clixon_xml_parselval.string = yytext;return (CHARDATA);} "-->" { BEGIN(START); return ECOMMENT; } . diff --git a/util/clixon_util_xpath.c b/util/clixon_util_xpath.c index de760508..8ca280d5 100644 --- a/util/clixon_util_xpath.c +++ b/util/clixon_util_xpath.c @@ -296,7 +296,11 @@ main(int argc, if (ret > 0 && (ret = xml_yang_validate_add(h, x1, &xerr)) < 0) goto done; if (ret == 0){ - if (netconf_err2cb(xerr, &cbret) < 0) + if ((cbret = cbuf_new()) ==NULL){ + clicon_err(OE_XML, errno, "cbuf_new"); + goto done; + } + if (netconf_err2cb(xerr, cbret) < 0) goto done; fprintf(stderr, "xml validation error: %s\n", cbuf_get(cbret)); goto done;