From 8c36083e1693fc98c183ed0ee944af382a014dc2 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Fri, 19 Apr 2019 16:01:39 +0200 Subject: [PATCH] * A yang type regex cache added, this helps the performance by avoiding re-running the `regcomp` command on every iteration. * An XML namespace cache added (see `xml2ns()`) * Better performance of XML whitespace parsing/scanning. --- apps/backend/backend_commit.c | 7 ++- apps/backend/backend_startup.c | 8 +++ lib/clixon/clixon_yang.h | 4 ++ lib/src/clixon_xml.c | 13 +++- lib/src/clixon_xml_parse.l | 6 +- lib/src/clixon_xml_parse.y | 56 +++++++++++++++-- lib/src/clixon_yang.c | 27 ++++++++ lib/src/clixon_yang_type.c | 112 ++++++++++++++++++++++++++++++--- 8 files changed, 214 insertions(+), 19 deletions(-) diff --git a/apps/backend/backend_commit.c b/apps/backend/backend_commit.c index a5fe8845..3565ea83 100644 --- a/apps/backend/backend_commit.c +++ b/apps/backend/backend_commit.c @@ -146,7 +146,7 @@ generic_validate(yang_stmt *yspec, * and call application callback validations. * @param[in] h Clicon handle * @param[in] db The startup database. The wanted backend state - * @param[out] xtr Transformed XML + * @param[in] td Transaction * @param[out] cbret CLIgen buffer w error stmt if retval = 0 * @retval -1 Error - or validation failed (but cbret not set) * @retval 0 Validation failed (with cbret set) @@ -183,6 +183,10 @@ startup_common(clicon_handle h, clicon_debug(1, "Reading startup config from %s", db); if (xmldb_get(h, db, "/", &xt, msd) < 0) goto done; + if (xml_child_nr(xt) == 0){ /* If empty skip */ + td->td_target = xt; + goto ok; + } if (msd){ if ((ret = clixon_module_upgrade(h, xt, msd, cbret)) < 0) goto done; @@ -225,6 +229,7 @@ startup_common(clicon_handle h, /* 7. Call plugin transaction complete callbacks */ if (plugin_transaction_complete(h, td) < 0) goto done; + ok: retval = 1; done: if (msd) diff --git a/apps/backend/backend_startup.c b/apps/backend/backend_startup.c index fe0d5ba8..34102e7a 100644 --- a/apps/backend/backend_startup.c +++ b/apps/backend/backend_startup.c @@ -114,6 +114,7 @@ db_merge(clicon_handle h, /*! Clixon startup startup mode: Commit startup configuration into running state * @param[in] h Clixon handle + * @param[in] db tmp or startup * @param[out] cbret If status is invalid contains error message * @retval -1 Error * @retval 0 Validation failed @@ -150,6 +151,10 @@ startup_mode_startup(clicon_handle h, int retval = -1; int ret; + if (strcmp(db, "running")==0){ + clicon_err(OE_FATAL, 0, "Invalid startup db: %s", db); + goto done; + } /* Load plugins and call plugin_init() */ if (backend_plugin_initiate(h) != 0) goto done; @@ -255,6 +260,8 @@ startup_extraxml(clicon_handle h, goto done; if (ret == 0) goto fail; + if (xt==NULL /* || xml_child_nr(xt)==0 */ ) /* This gives SEGV in test_feature */ + goto ok; /* Write (potentially modified) xml tree xt back to tmp */ if ((ret = xmldb_put(h, "tmp", OP_REPLACE, xt, @@ -265,6 +272,7 @@ startup_extraxml(clicon_handle h, goto fail; if (ret == 0) goto fail; + ok: retval = 1; done: if (xt) diff --git a/lib/clixon/clixon_yang.h b/lib/clixon/clixon_yang.h index f9472bba..81153b72 100644 --- a/lib/clixon/clixon_yang.h +++ b/lib/clixon/clixon_yang.h @@ -218,7 +218,9 @@ struct yang_stmt{ Y_TYPE & identity: store all derived types */ yang_type_cache *ys_typecache; /* If ys_keyword==Y_TYPE, cache all typedef data except unions */ + void *ys_regex_cache; /* regex cache */ int _ys_vector_i; /* internal use: yn_each */ + }; typedef int (yang_applyfn_t)(yang_stmt *ys, void *arg); @@ -232,6 +234,8 @@ enum rfc_6020 yang_keyword_get(yang_stmt *ys); char *yang_argument_get(yang_stmt *ys); cg_var *yang_cv_get(yang_stmt *ys); cvec *yang_cvec_get(yang_stmt *ys); +void *yang_regex_cache_get(yang_stmt *ys); +int yang_regex_cache_set(yang_stmt *ys, void *regex); /* Other functions */ yang_stmt *yspec_new(void); diff --git a/lib/src/clixon_xml.c b/lib/src/clixon_xml.c index b19909a6..50f6f7e7 100644 --- a/lib/src/clixon_xml.c +++ b/lib/src/clixon_xml.c @@ -127,6 +127,7 @@ struct xml{ reference, dont free */ cg_var *x_cv; /* Cached value as cligen variable (eg xml_cmp) */ + char *x_ns_cache; /* Cached namespace */ int _x_vector_i; /* internal use: xml_child_each */ int _x_i; /* internal use for sorting: see xml_enumerate and xml_cmp */ @@ -234,6 +235,7 @@ xml_prefix_set(cxobj *xn, * @retval 0 OK * @retval -1 Error * @see xmlns_check XXX can these be merged? + * @note, this function uses a cache. Any case where cache should be cleared? */ int xml2ns(cxobj *x, @@ -241,9 +243,11 @@ xml2ns(cxobj *x, char **namespace) { int retval = -1; - char *ns; + char *ns = NULL; cxobj *xp; + if ((ns = x->x_ns_cache) != NULL) + goto ok; if (prefix != NULL) /* xmlns:="" */ ns = xml_find_type_value(x, "xmlns", prefix, CX_ATTR); else /* xmlns="" */ @@ -261,6 +265,11 @@ xml2ns(cxobj *x, ns = DEFAULT_XML_RPC_NAMESPACE; #endif } + if (ns && (x->x_ns_cache = strdup(ns)) == NULL){ + clicon_err(OE_XML, errno, "strdup"); + goto done; + } + ok: if (namespace) *namespace = ns; retval = 0; @@ -1359,6 +1368,8 @@ xml_free(cxobj *x) free(x->x_childvec); if (x->x_cv) cv_free(x->x_cv); + if (x->x_ns_cache) + free(x->x_ns_cache); free(x); return 0; } diff --git a/lib/src/clixon_xml_parse.l b/lib/src/clixon_xml_parse.l index 35a9d5af..8d1826c2 100644 --- a/lib/src/clixon_xml_parse.l +++ b/lib/src/clixon_xml_parse.l @@ -132,10 +132,10 @@ ncname {namestart}{namechar}* "\< { BEGIN(START); return *clixon_xml_parsetext; } & { _YA->ya_lex_state =STATEA;BEGIN(AMPERSAND);} -[ \t] { clixon_xml_parselval.string = yytext;return WHITESPACE; } -\r\n { clixon_xml_parselval.string = "\n";return WHITESPACE; } +[ \t]+ { clixon_xml_parselval.string = yytext;return WHITESPACE; } +\r\n { clixon_xml_parselval.string = "\n"; _YA->ya_linenum++; return WHITESPACE; } \r { clixon_xml_parselval.string = "\n";return WHITESPACE; } -\n { clixon_xml_parselval.string = yytext; _YA->ya_linenum++;return WHITESPACE; } +\n { clixon_xml_parselval.string = "\n"; _YA->ya_linenum++;return WHITESPACE; } . { clixon_xml_parselval.string = yytext; return CHARDATA; } /* @see xml_chardata_encode */ diff --git a/lib/src/clixon_xml_parse.y b/lib/src/clixon_xml_parse.y index b5fcbc05..96876021 100644 --- a/lib/src/clixon_xml_parse.y +++ b/lib/src/clixon_xml_parse.y @@ -112,6 +112,44 @@ xml_parse_content(struct xml_parse_yacc_arg *ya, return retval; } +/*! Add whitespace + * If text, ie only body, keep as is. + * But if there is an element, then skip all whitespace. + */ +static int +xml_parse_whitespace(struct xml_parse_yacc_arg *ya, + char *str) +{ + cxobj *xn = ya->ya_xelement; + cxobj *xp = ya->ya_xparent; + int retval = -1; + int i; + + ya->ya_xelement = NULL; /* init */ + /* If there is an element already, only add one whitespace child + * otherwise, keep all whitespace. + */ +#if 1 + for (i=0; iya_xelement = xn; + ok: + retval = 0; + done: + return retval; +} + + static int xml_parse_version(struct xml_parse_yacc_arg *ya, char *ver) @@ -243,11 +281,17 @@ xml_parse_bslash1(struct xml_parse_yacc_arg *ya, while ((xc = xml_child_each(x, xc, CX_ELMNT)) != NULL) break; if (xc != NULL){ /* at least one element */ - xc = NULL; - while ((xc = xml_child_each(x, xc, CX_BODY)) != NULL) { - xml_purge(xc); - xc = NULL; /* reset iterator */ - } + int i; + for (i=0; i element"); } | pi { clicon_debug(2, "content -> pi"); } | CHARDATA { if (xml_parse_content(_YA, $1) < 0) YYABORT; clicon_debug(2, "content -> CHARDATA %s", $1); } - | WHITESPACE { if (xml_parse_content(_YA, $1) < 0) YYABORT; + | WHITESPACE { if (xml_parse_whitespace(_YA, $1) < 0) YYABORT; clicon_debug(2, "content -> WHITESPACE %s", $1); } | { clicon_debug(2, "content -> "); } ; diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c index 5003c1a5..0e7d443b 100644 --- a/lib/src/clixon_yang.c +++ b/lib/src/clixon_yang.c @@ -69,6 +69,7 @@ #include #include #include +#include /* cligen */ #include @@ -222,6 +223,20 @@ yang_cvec_get(yang_stmt *ys) return ys->ys_cvec; } +void* +yang_regex_cache_get(yang_stmt *ys) +{ + return ys->ys_regex_cache; +} + +int +yang_regex_cache_set(yang_stmt *ys, + void *regex) +{ + ys->ys_regex_cache = regex; + return 0; +} + /* End access functions */ /*! Create new yang specification @@ -266,6 +281,17 @@ ys_new(enum rfc_6020 keyw) return ys; } + +static int +yang_regex_cache_free(yang_stmt *ys) +{ + if (ys->ys_regex_cache){ + regfree(ys->ys_regex_cache); + free(ys->ys_regex_cache); + } + return 0; +} + /*! Free a single yang statement */ static int ys_free1(yang_stmt *ys) @@ -280,6 +306,7 @@ ys_free1(yang_stmt *ys) cvec_free(ys->ys_cvec); if (ys->ys_typecache) yang_type_cache_free(ys->ys_typecache); + yang_regex_cache_free(ys); free(ys); return 0; } diff --git a/lib/src/clixon_yang_type.c b/lib/src/clixon_yang_type.c index 413d4e8e..483ed0ac 100644 --- a/lib/src/clixon_yang_type.c +++ b/lib/src/clixon_yang_type.c @@ -49,6 +49,7 @@ #include #include #include +#include #include /* cligen */ @@ -99,6 +100,74 @@ static const map_str2int ytmap[] = { {NULL, -1} }; +/*! Regular expression compiling + * @retval -1 Error + * @retval 0 regex problem (no match?) + * @retval 1 OK Match + * @see match_regexp the CLIgen original composite function + */ +static int +regex_compile(char *pattern0, + regex_t *re) +{ + int retval = -1; + char pattern[1024]; + // char errbuf[1024]; + int len0; + int status; + + len0 = strlen(pattern0); + if (len0 > sizeof(pattern)-5){ + clicon_err(OE_XML, EINVAL, "pattern too long"); + goto done; + } + strncpy(pattern, "^(", 2); + strncpy(pattern+2, pattern0, sizeof(pattern)-2); + strncat(pattern, ")$", sizeof(pattern)-len0-1); + if ((status = regcomp(re, pattern, REG_NOSUB|REG_EXTENDED)) != 0) { +#if 0 /* ignore error msg for now */ + regerror(status, re, errbuf, sizeof(errbuf)); +#endif + goto fail; + } + retval = 1; + done: + return retval; + fail: + retval = 0; + goto done; +} + +/*! Regular expression execution + * @retval -1 Error + * @retval 0 regex problem (no match?) + * @retval 1 OK Match + * @see match_regexp the CLIgen original composite function + */ +static int +regex_exec(regex_t *re, + char *string) +{ + int retval = -1; + int status; + // char errbuf[1024]; + + status = regexec(re, string, (size_t) 0, NULL, 0); + if (status != 0) { +#if 0 /* ignore error msg for now */ + regerror(status, re, errbuf, sizeof(errbuf)); +#endif + goto fail; + } + retval = 1; + done: + return retval; + fail: + retval = 0; + goto done; +} + + /* return 1 if built-in, 0 if not */ static int yang_builtin(char *type) @@ -525,14 +594,41 @@ cv_validate1(cg_var *cv, } if ((options & YANG_OPTIONS_PATTERN) != 0){ char *posix = NULL; - if (regexp_xsd2posix(pattern, &posix) < 0) - goto done; - if ((retval2 = match_regexp(str?str:"", posix)) < 0){ - clicon_err(OE_DB, 0, "match_regexp: %s", pattern); - return -1; + regex_t *re = NULL; + + if ((re = yang_regex_cache_get(yrestype)) == NULL){ + /* Transform to posix regex */ + if (regexp_xsd2posix(pattern, &posix) < 0) + goto done; + /* Create regex cache */ + if ((re = malloc(sizeof(*re))) == NULL){ + clicon_err(OE_UNIX, errno, "malloc"); + goto done; + } + memset(re, 0, sizeof(*re)); + /* Compute regex pattern for use in patterns */ + if ((retval2 = regex_compile(posix, re)) < 0) + goto done; + if (retval2 == 0){ + if (reason) + *reason = cligen_reason("regexp match fail: \"%s\" does not match %s", + str, pattern); + goto fail; + break; + } + yang_regex_cache_set(yrestype, re); + if (posix) + free(posix); + } + if ((retval2 = regex_exec(re, str?str:"")) < 0) + goto done; + if (retval2 == 0){ + if (reason) + *reason = cligen_reason("regexp match fail: \"%s\" does not match %s", + str, pattern); + goto fail; + break; } - if (posix) - free(posix); if (retval2 == 0){ if (reason) *reason = cligen_reason("regexp match fail: \"%s\" does not match %s", @@ -667,7 +763,7 @@ ys_cv_validate_union(yang_stmt *ys, /*! Validate cligen variable cv using yang statement as spec * * @param[in] cv A cligen variable to validate. This is a correctly parsed cv. - * @param[in] ys A yang statement, must be leaf of leaf-list. + * @param[in] ys A yang statement, must be leaf or leaf-list. * @param[out] reason If given, and if return value is 0, contains a malloced string * describing the reason why the validation failed. Must be freed. * @retval -1 Error (fatal), with errno set to indicate error