From 4742fde1e9736c5e2af7bc59c183acb3891e5ead Mon Sep 17 00:00:00 2001 From: Olof Hagsand Date: Sat, 9 Dec 2017 14:14:40 +0100 Subject: [PATCH] XML creation and parse API changes --- CHANGELOG.md | 19 ++- apps/backend/backend_main.c | 6 +- apps/cli/cli_common.c | 2 +- apps/netconf/netconf_lib.c | 2 +- apps/netconf/netconf_main.c | 2 +- apps/netconf/netconf_plugin.c | 73 +--------- apps/netconf/netconf_rpc.c | 128 ++++++++++++++--- apps/restconf/restconf_methods.c | 6 +- datastore/datastore_client.c | 2 +- datastore/text/clixon_xmldb_text.c | 6 +- example/routing_backend.c | 12 +- example/routing_cli.c | 2 +- lib/clixon/clixon_xml.h | 22 ++- lib/src/clixon_options.c | 2 +- lib/src/clixon_proto.c | 2 +- lib/src/clixon_proto_client.c | 2 +- lib/src/clixon_xml.c | 218 +++++++++++++++-------------- lib/src/clixon_xml_db.c | 2 +- lib/src/clixon_xsl.c | 2 +- lib/src/json_xpath.c | 2 +- test/test_cli.sh | 55 ++++---- 21 files changed, 309 insertions(+), 258 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b3c9516..7e1eb67a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,16 +12,15 @@ * Moved XML_CHILD_HASH to variable instead of constant. -* Added yang to XML API - - * xml_new(char *name, cxobj *xn_parent) --> xml_new(char *name, cxobj *xn_parent, void *spec) - * xml_new_spec(char *name, cxobj *xn_parent, void *spec) --> xml_new(char *name, cxobj *xn_parent, void *spec) - * clicon_xml_parse_string --> clicon_xml_parse_str - * clicon_xml_parse_file(int fd, cxobj **xml_top, char *endtag) --> clicon_xml_parse_file(int fd, char *endtag, yang_spec *yspec, cxobj **xt) - * clicon_xml_parse_str(char *str, cxobj **xml_top) --> clicon_xml_parse_str(char *str, yang_spec *yspec, cxobj **xml_top) - * clicon_xml_parse(cxobj **cxtop, char *format, ...) --> clicon_xml_parse(cxobj **cxtop, yang_spec *yspec, char *format, ...) - * xml_parse(char *str, cxobj *xt) --> xml_parse(char *str, yang_spec *yspec, cxobj *xt); - +* Changed XML creation and parse API for more coherency and closer YANG/XML integration. A new yang spec parameter has been added (default NULL) and functions have been removed and renamed. You may need to change the XML calls as follows. + * xml_new(name, parent) --> xml_new(name, xn_parent, yspec) + * xml_new_spec(name, parent, spec) --> xml_new(name, parent, spec) + * clicon_xml_parse_file(fd, &xt, endtag) --> xml_parse_file(fd, endtag, yspec, &xt) + * clicon_xml_parse_string(&str, &xt) --> xml_parse_string(str, yspec, &xt) + * xml_parse(str, xt) --> xml_parse_string(str, yspec, &xt) + * clicon_xml_parse(&xt, format, ...) --> xml_parse_va(&xt, yspec, format, ...) + * clicon_xml_parse_str(str, &xt) --> xml_parse_string(str, yspec, &xt) + ## 3.3.3 (25 November 2017) Thanks to Matthew Smith, Joe Loeliger at Netgate; Fredrik Pettai at diff --git a/apps/backend/backend_main.c b/apps/backend/backend_main.c index 022bcb81..927749e1 100644 --- a/apps/backend/backend_main.c +++ b/apps/backend/backend_main.c @@ -296,7 +296,7 @@ rundb_main(clicon_handle h, clicon_err(OE_UNIX, errno, "open(%s)", extraxml_file); goto done; } - if (clicon_xml_parse_file(fd, &xt, "") < 0) + if (xml_parse_file(fd, &xt, "") < 0) goto done; if ((xn = xml_child_i(xt, 0)) != NULL) if (xmldb_put(h, "tmp", OP_MERGE, xn) < 0) @@ -442,7 +442,7 @@ load_extraxml(clicon_handle h, clicon_err(OE_UNIX, errno, "open(%s)", filename); goto done; } - if (clicon_xml_parse_file(fd, "", NULL, &xt) < 0) + if (xml_parse_file(fd, "", NULL, &xt) < 0) goto done; /* Replace parent w first child */ if (xml_rootchild(xt, 0, &xt) < 0) @@ -827,7 +827,7 @@ main(int argc, char **argv) if (xmldb_setopt(h, "yangspec", clicon_dbspec_yang(h)) < 0) goto done; if ((xml_cache = clicon_option_bool(h, "CLICON_XMLDB_CACHE")) >= 0) - if (xmldb_setopt(h, "xml_cache", (void*)xml_cache) < 0) + if (xmldb_setopt(h, "xml_cache", (void*)(intptr_t)xml_cache) < 0) goto done; /* If startup mode is not defined, eg via OPTION or -s, assume old method */ startup_mode = clicon_startup_mode(h); diff --git a/apps/cli/cli_common.c b/apps/cli/cli_common.c index d7de2953..cd063168 100644 --- a/apps/cli/cli_common.c +++ b/apps/cli/cli_common.c @@ -746,7 +746,7 @@ load_config_file(clicon_handle h, clicon_err(OE_UNIX, errno, "%s: open(%s)", __FUNCTION__, filename); goto done; } - if (clicon_xml_parse_file(fd, "", NULL, &xt) < 0) + if (xml_parse_file(fd, "", NULL, &xt) < 0) goto done; if (xt == NULL) goto done; diff --git a/apps/netconf/netconf_lib.c b/apps/netconf/netconf_lib.c index a6d8e87e..b3edcc73 100644 --- a/apps/netconf/netconf_lib.c +++ b/apps/netconf/netconf_lib.c @@ -190,7 +190,7 @@ netconf_output(int s, clicon_debug(1, "SEND %s", msg); if (debug > 1){ /* XXX: below only works to stderr, clicon_debug may log to syslog */ cxobj *xt = NULL; - if (clicon_xml_parse_str(buf, NULL, &xt) == 0){ + if (xml_parse_string(buf, NULL, &xt) == 0){ clicon_xml2file(stderr, xml_child_i(xt, 0), 0, 0); fprintf(stderr, "\n"); xml_free(xt); diff --git a/apps/netconf/netconf_main.c b/apps/netconf/netconf_main.c index 3326c69d..cb97cf9b 100644 --- a/apps/netconf/netconf_main.c +++ b/apps/netconf/netconf_main.c @@ -98,7 +98,7 @@ process_incoming_packet(clicon_handle h, } str = str0; /* Parse incoming XML message */ - if (clicon_xml_parse_str(str, NULL, &xreq) < 0){ + if (xml_parse_string(str, NULL, &xreq) < 0){ if ((cbret = cbuf_new()) == NULL){ cprintf(cbret, "" "operation-failed" diff --git a/apps/netconf/netconf_plugin.c b/apps/netconf/netconf_plugin.c index 205d8124..30473ad1 100644 --- a/apps/netconf/netconf_plugin.c +++ b/apps/netconf/netconf_plugin.c @@ -198,6 +198,9 @@ catch: } /*! See if there is any callback registered for this tag + * + * Look for local (client-side) netconf plugins. This feature may no + * longer be necessary as generic RPC:s should be handled by backend. * * @param[in] h clicon handle * @param[in] xn Sub-tree (under xorig) at child of rpc: . @@ -214,13 +217,7 @@ netconf_plugin_callbacks(clicon_handle h, { int retval = -1; netconf_reg_t *nreg; - yang_spec *yspec = NULL; /* application yspec */ - yang_stmt *yrpc; - yang_stmt *yinput; - yang_stmt *youtput; - cxobj *xoutput; - cbuf *cb = NULL; - + if (deps != NULL){ nreg = deps; do { @@ -233,70 +230,8 @@ netconf_plugin_callbacks(clicon_handle h, nreg = NEXTQ(netconf_reg_t *, nreg); } while (nreg != deps); } - /* First check system / netconf RPC:s */ - if ((cb = cbuf_new()) == NULL){ - clicon_err(OE_UNIX, 0, "cbuf_new"); - goto done; - } - /* create absolute path */ - if (xml_namespace(xn)) - cprintf(cb, "/%s:%s", xml_namespace(xn), xml_name(xn)); - else - cprintf(cb, "/nc:%s", xml_name(xn)); /* Hardcoded for netconf */ - /* Find yang rpc statement, return yang rpc statement if found */ - if (yrpc == NULL){ - /* Then check application RPC */ - if ((yspec = clicon_dbspec_yang(h)) == NULL){ - clicon_err(OE_YANG, ENOENT, "No yang spec"); - goto done; - } - cbuf_reset(cb); - if (xml_namespace(xn)) - cprintf(cb, "/%s:%s", xml_namespace(xn), xml_name(xn)); - else - cprintf(cb, "/%s", xml_name(xn)); /* XXX not accepdted by below */ - /* Find yang rpc statement, return yang rpc statement if found */ - if (yang_abs_schema_nodeid(yspec, cbuf_get(cb), &yrpc) < 0) - goto done; - } - /* Check if found */ - if (yrpc != NULL){ - if ((yinput = yang_find((yang_node*)yrpc, Y_INPUT, NULL)) != NULL){ - xml_spec_set(xn, yinput); /* needed for xml_spec_populate */ - if (xml_apply(xn, CX_ELMNT, xml_spec_populate, yinput) < 0) - goto done; - if (xml_apply(xn, CX_ELMNT, - (xml_applyfn_t*)xml_yang_validate_all, NULL) < 0) - goto done; - if (xml_yang_validate_add(xn, NULL) < 0) - goto done; - } - /* - * 1. Check xn arguments with input statement. - * 2. Send to backend as clicon_msg-encode() - * 3. In backend to similar but there call actual backend - */ - if (clicon_rpc_netconf_xml(h, xml_parent(xn), xret, NULL) < 0) - goto done; - /* Sanity check of outgoing XML */ - if ((youtput = yang_find((yang_node*)yrpc, Y_OUTPUT, NULL)) != NULL){ - xoutput=xpath_first(*xret, "/"); - xml_spec_set(xoutput, youtput); /* needed for xml_spec_populate */ - if (xml_apply(xoutput, CX_ELMNT, xml_spec_populate, youtput) < 0) - goto done; - if (xml_apply(xoutput, CX_ELMNT, - (xml_applyfn_t*)xml_yang_validate_all, NULL) < 0) - goto done; - if (xml_yang_validate_add(xoutput, NULL) < 0) - goto done; - } - retval = 1; /* handled by callback */ - goto done; - } retval = 0; done: - if (cb) - cbuf_free(cb); return retval; } diff --git a/apps/netconf/netconf_rpc.c b/apps/netconf/netconf_rpc.c index 7e2d86ba..a912d0ed 100644 --- a/apps/netconf/netconf_rpc.c +++ b/apps/netconf/netconf_rpc.c @@ -135,7 +135,7 @@ netconf_get_config(clicon_handle h, cxobj *xconf; if ((source = netconf_get_target(xn, "source")) == NULL){ - clicon_xml_parse(xret, NULL, "" + xml_parse_va(xret, NULL, "" "missing-element" "protocol" "error" @@ -163,7 +163,7 @@ netconf_get_config(clicon_handle h, /* xml_filter removes parts of xml tree not matching */ if ((strcmp(xml_name(xfilterconf), xml_name(xconf))!=0) || xml_filter(xfilterconf, xconf) < 0){ - clicon_xml_parse(xret, NULL, "" + xml_parse_va(xret, NULL, "" "operation-failed" "applicatio" "error" @@ -173,7 +173,7 @@ netconf_get_config(clicon_handle h, } } else{ - clicon_xml_parse(xret, NULL, "" + xml_parse_va(xret, NULL, "" "operation-failed" "applicatio" "error" @@ -241,7 +241,7 @@ get_edit_opts(cxobj *xn, retval = 1; /* hunky dory */ return retval; parerr: /* parameter error, xret set */ - clicon_xml_parse(xret, NULL, "" + xml_parse_va(xret, NULL, "" "invalid-value" "protocol" "error" @@ -317,7 +317,7 @@ netconf_edit_config(clicon_handle h, /* must have target, and it should be candidate */ if ((target = netconf_get_target(xn, "target")) == NULL || strcmp(target, "candidate")){ - clicon_xml_parse(xret, NULL, "" + xml_parse_va(xret, NULL, "" "missing-element" "protocol" "error" @@ -329,7 +329,7 @@ netconf_edit_config(clicon_handle h, if ((xfilter = xpath_first(xn, "filter")) != NULL) { if ((ftype = xml_find_value(xfilter, "type")) != NULL) if (strcmp(ftype,"restconf")){ - clicon_xml_parse(xret, NULL, "" + xml_parse_va(xret, NULL, "" "invalid-value" "protocol" "error" @@ -339,7 +339,7 @@ netconf_edit_config(clicon_handle h, } if ((x = xpath_first(xn, "default-operation")) != NULL){ if (xml_operation(xml_body(x), &operation) < 0){ - clicon_xml_parse(xret, NULL, "" + xml_parse_va(xret, NULL, "" "invalid-value" "protocol" "error" @@ -353,7 +353,7 @@ netconf_edit_config(clicon_handle h, goto ok; /* not supported opts */ if (testopt!=TEST_THEN_SET || erropt!=STOP_ON_ERROR){ - clicon_xml_parse(xret, NULL, "" + xml_parse_va(xret, NULL, "" "operation-not-supported" "protocol" "error" @@ -403,7 +403,7 @@ netconf_copy_config(clicon_handle h, char *target; /* filenames */ if ((source = netconf_get_target(xn, "source")) == NULL){ - clicon_xml_parse(xret, NULL, "" + xml_parse_va(xret, NULL, "" "missing-element" "protocol" "error" @@ -412,7 +412,7 @@ netconf_copy_config(clicon_handle h, goto ok; } if ((target = netconf_get_target(xn, "target")) == NULL){ - clicon_xml_parse(xret, NULL, "" + xml_parse_va(xret, NULL, "" "missing-element" "protocol" "error" @@ -450,7 +450,7 @@ netconf_delete_config(clicon_handle h, if ((target = netconf_get_target(xn, "target")) == NULL || strcmp(target, "running")==0){ - clicon_xml_parse(xret, NULL, "" + xml_parse_va(xret, NULL, "" "missing-element" "protocol" "error" @@ -486,7 +486,7 @@ netconf_lock(clicon_handle h, char *target; if ((target = netconf_get_target(xn, "target")) == NULL){ - clicon_xml_parse(xret, NULL, "" + xml_parse_va(xret, NULL, "" "missing-element" "protocol" "error" @@ -566,7 +566,7 @@ netconf_get(clicon_handle h, /* xml_filter removes parts of xml tree not matching */ if ((strcmp(xml_name(xfilterconf), xml_name(xconf))!=0) || xml_filter(xfilterconf, xconf) < 0){ - clicon_xml_parse(xret, NULL, "" + xml_parse_va(xret, NULL, "" "operation-failed" "applicatio" "error" @@ -576,7 +576,7 @@ netconf_get(clicon_handle h, } } else{ - clicon_xml_parse(xret, NULL, "" + xml_parse_va(xret, NULL, "" "operation-failed" "applicatio" "error" @@ -627,7 +627,7 @@ netconf_kill_session(clicon_handle h, cxobj *xs; if ((xs = xpath_first(xn, "//session-id")) == NULL){ - clicon_xml_parse(xret, NULL, "" + xml_parse_va(xret, NULL, "" "missing-element" "protocol" "error" @@ -658,7 +658,7 @@ netconf_validate(clicon_handle h, char *target; if ((target = netconf_get_target(xn, "source")) == NULL){ - clicon_xml_parse(xret, NULL, "" + xml_parse_va(xret, NULL, "" "missing-element" "protocol" "error" @@ -826,7 +826,7 @@ netconf_create_subscription(clicon_handle h, if ((xfilter = xpath_first(xn, "//filter")) != NULL){ if ((ftype = xml_find_value(xfilter, "type")) != NULL){ if (strcmp(ftype, "xpath") != 0){ - clicon_xml_parse(xret, NULL, "" + xml_parse_va(xret, NULL, "" "operation-failed" "application" "error" @@ -850,6 +850,92 @@ netconf_create_subscription(clicon_handle h, return retval; } +/*! See if there is any callback registered for this tag + * + * @param[in] h clicon handle + * @param[in] xn Sub-tree (under xorig) at child of rpc: . + * @param[out] xret Return XML, error or OK + * + * @retval -1 Error + * @retval 0 OK, not found handler. + * @retval 1 OK, handler called + */ +static int +netconf_application_rpc(clicon_handle h, + cxobj *xn, + cxobj **xret) +{ + int retval = -1; + yang_spec *yspec = NULL; /* application yspec */ + yang_stmt *yrpc = NULL; + yang_stmt *yinput; + yang_stmt *youtput; + cxobj *xoutput; + cbuf *cb = NULL; + + /* First check system / netconf RPC:s */ + if ((cb = cbuf_new()) == NULL){ + clicon_err(OE_UNIX, 0, "cbuf_new"); + goto done; + } + /* Find yang rpc statement, return yang rpc statement if found + Check application RPC */ + if ((yspec = clicon_dbspec_yang(h)) == NULL){ + clicon_err(OE_YANG, ENOENT, "No yang spec"); + goto done; + } + cbuf_reset(cb); + // if (xml_namespace(xn)) + cprintf(cb, "/%s:%s", xml_namespace(xn), xml_name(xn)); + // else + // cprintf(cb, "/%s", xml_name(xn)); /* XXX not accepdted by below */ + /* Find yang rpc statement, return yang rpc statement if found */ + if (yang_abs_schema_nodeid(yspec, cbuf_get(cb), &yrpc) < 0) + goto done; + + /* Check if found */ + if (yrpc != NULL){ + if ((yinput = yang_find((yang_node*)yrpc, Y_INPUT, NULL)) != NULL){ + xml_spec_set(xn, yinput); /* needed for xml_spec_populate */ + if (xml_apply(xn, CX_ELMNT, xml_spec_populate, yinput) < 0) + goto done; + if (xml_apply(xn, CX_ELMNT, + (xml_applyfn_t*)xml_yang_validate_all, NULL) < 0) + goto done; + if (xml_yang_validate_add(xn, NULL) < 0) + goto done; + } + /* + * 1. Check xn arguments with input statement. + * 2. Send to backend as clicon_msg-encode() + * 3. In backend to similar but there call actual backend + */ + if (clicon_rpc_netconf_xml(h, xml_parent(xn), xret, NULL) < 0) + goto done; + /* Sanity check of outgoing XML */ + if ((youtput = yang_find((yang_node*)yrpc, Y_OUTPUT, NULL)) != NULL){ + xoutput=xpath_first(*xret, "/"); + xml_spec_set(xoutput, youtput); /* needed for xml_spec_populate */ + if (xml_apply(xoutput, CX_ELMNT, xml_spec_populate, youtput) < 0) + goto done; + if (xml_apply(xoutput, CX_ELMNT, + (xml_applyfn_t*)xml_yang_validate_all, NULL) < 0) + goto done; + if (xml_yang_validate_add(xoutput, NULL) < 0) + goto done; + } + retval = 1; /* handled by callback */ + goto done; + } + retval = 0; + done: + if (cb) + cbuf_free(cb); + return retval; +} + + + /*! The central netconf rpc dispatcher. Look at first tag and dispach to sub-functions. * Call plugin handler if tag not found. If not handled by any handler, return * error. @@ -932,10 +1018,16 @@ netconf_rpc_dispatch(clicon_handle h, } /* Others */ else { + /* Look for local (client-side) netconf plugins. This feature may no + * longer be necessary as generic RPC:s should be handled by backend. + */ if ((retval = netconf_plugin_callbacks(h, xe, xret)) < 0) goto done; + if (retval == 0) + if ((retval = netconf_application_rpc(h, xe, xret)) < 0) + goto done; if (retval == 0){ /* not handled by callback */ - clicon_xml_parse(xret, NULL, "" + xml_parse_va(xret, NULL, "" "operation-failed" "rpc" "error" diff --git a/apps/restconf/restconf_methods.c b/apps/restconf/restconf_methods.c index bd078216..67cbbad0 100644 --- a/apps/restconf/restconf_methods.c +++ b/apps/restconf/restconf_methods.c @@ -376,7 +376,7 @@ api_data_post(clicon_handle h, goto done; /* Parse input data as json or xml into xml */ if (parse_xml){ - if (clicon_xml_parse_str(data, NULL, &xdata) < 0){ + if (xml_parse_string(data, NULL, &xdata) < 0){ clicon_debug(1, "%s json parse fail: %s", __FUNCTION__, data); goto done; } @@ -495,7 +495,7 @@ api_data_put(clicon_handle h, goto done; /* Parse input data as json or xml into xml */ if (parse_xml){ - if (clicon_xml_parse_str(data, NULL, &xdata) < 0){ + if (xml_parse_string(data, NULL, &xdata) < 0){ clicon_debug(1, "%s json parse fail: %s", __FUNCTION__, data); goto done; } @@ -729,7 +729,7 @@ api_operation_post(clicon_handle h, if (data && strlen(data)){ /* Parse input data as json or xml into xml */ if (parse_xml){ - if (clicon_xml_parse_str(data, NULL, &xdata) < 0){ + if (xml_parse_string(data, NULL, &xdata) < 0){ clicon_debug(1, "%s json parse fail: %s", __FUNCTION__, data); goto done; } diff --git a/datastore/datastore_client.c b/datastore/datastore_client.c index 3c9bcaeb..bd893f0b 100644 --- a/datastore/datastore_client.c +++ b/datastore/datastore_client.c @@ -257,7 +257,7 @@ main(int argc, char **argv) clicon_err(OE_DB, 0, "Unrecognized operation: %s", argv[1]); usage(argv0); } - if (clicon_xml_parse_str(argv[2], NULL, &xt) < 0) + if (xml_parse_string(argv[2], NULL, &xt) < 0) goto done; if (xml_rootchild(xt, 0, &xt) < 0) goto done; diff --git a/datastore/text/clixon_xmldb_text.c b/datastore/text/clixon_xmldb_text.c index 11b65e54..d45dca93 100644 --- a/datastore/text/clixon_xmldb_text.c +++ b/datastore/text/clixon_xmldb_text.c @@ -431,7 +431,7 @@ text_get(xmldb_handle xh, goto done; } /* Parse file into XML tree */ - if ((clicon_xml_parse_file(fd, "", yspec, &xt)) < 0) + if ((xml_parse_file(fd, "", yspec, &xt)) < 0) goto done; /* Always assert a top-level called "config". To ensure that, deal with two cases: @@ -856,7 +856,7 @@ text_put(xmldb_handle xh, goto done; } /* Parse file into XML tree */ - if ((clicon_xml_parse_file(fd, "", yspec, &x0)) < 0) + if ((xml_parse_file(fd, "", yspec, &x0)) < 0) goto done; /* Always assert a top-level called "config". To ensure that, deal with two cases: @@ -1345,7 +1345,7 @@ main(int argc, char **argv) if (strcmp(cmd, "put")==0){ if (argc != 6) usage(argv[0]); - if (clicon_xml_parse_file(0, "", NULL, &xt) < 0) + if (xml_parse_file(0, "", NULL, &xt) < 0) goto done; if (xml_rootchild(xt, 0, &xn) < 0) goto done; diff --git a/example/routing_backend.c b/example/routing_backend.c index da3d3e53..1c672b02 100644 --- a/example/routing_backend.c +++ b/example/routing_backend.c @@ -165,11 +165,11 @@ plugin_statedata(clicon_handle h, cxobj **xvec = NULL; /* Example of (static) statedata, real code would poll state */ - if (xml_parse("" - "eth0" - "eth" - "42" - "", NULL, xstate) < 0) + if (xml_parse_string("" + "eth0" + "eth" + "42" + "", NULL, &xstate) < 0) goto done; retval = 0; done: @@ -225,7 +225,7 @@ plugin_reset(clicon_handle h, int retval = -1; cxobj *xt = NULL; - if (clicon_xml_parse_str("" + if (xml_parse_string("" "lolocal" "", NULL, &xt) < 0) goto done; diff --git a/example/routing_cli.c b/example/routing_cli.c index 6a6c26e4..c1e794ca 100644 --- a/example/routing_cli.c +++ b/example/routing_cli.c @@ -107,7 +107,7 @@ fib_route_rpc(clicon_handle h, /* User supplied variable in CLI command */ instance = cvec_find(cvv, "instance"); /* get a cligen variable from vector */ /* Create XML for fib-route netconf RPC */ - if (clicon_xml_parse(&xtop, NULL, "%s", instance) < 0) + if (xml_parse_va(&xtop, NULL, "%s", instance) < 0) goto done; /* Skip top-level */ xrpc = xml_child_i(xtop, 0); diff --git a/lib/clixon/clixon_xml.h b/lib/clixon/clixon_xml.h index de1dd636..d84cabca 100644 --- a/lib/clixon/clixon_xml.h +++ b/lib/clixon/clixon_xml.h @@ -113,6 +113,12 @@ cxobj *xml_child_each(cxobj *xparent, cxobj *xprev, enum cxobj_type type); cxobj **xml_childvec_get(cxobj *x); int xml_childvec_set(cxobj *x, int len); +#ifdef OBSOLETE /* Changed in 3.3.3, see CHANGELOG */ +/* Changed: */ +cxobj *xml_new(char *name, cxobj *xn_parent); +/* Removed: */ +cxobj *xml_new_spec(char *name, cxobj *xn_parent, void *spec); +#endif cxobj *xml_new(char *name, cxobj *xn_parent, yang_stmt *spec); yang_stmt *xml_spec(cxobj *x); int xml_spec_set(cxobj *x, yang_stmt *spec); @@ -136,12 +142,18 @@ int xml_free(cxobj *xn); int xml_print(FILE *f, cxobj *xn); int clicon_xml2file(FILE *f, cxobj *xn, int level, int prettyprint); int clicon_xml2cbuf(cbuf *xf, cxobj *xn, int level, int prettyprint); -int clicon_xml_parse_file(int fd, char *endtag, yang_spec *yspec, cxobj **xt); -int clicon_xml_parse_str(char *str, yang_spec *yspec, cxobj **xml_top); -/* XXX obsolete */ -#define clicon_xml_parse_string(str, x) clicon_xml_parse_str((*str), NULL, x) -int clicon_xml_parse(cxobj **cxtop, yang_spec *yspec, char *format, ...); +#ifdef OBSOLETE /* Changed in 3.3.3, see CHANGELOG */ +/* Changed: */ +int clicon_xml_parse(cxobj **cxtop, char *format, ...); +int clicon_xml_parse_file(int fd, cxobj **xml_top, char *endtag); +int clicon_xml_parse_str(char *str, cxobj **xml_top); +/* Removed: */ int xml_parse(char *str, yang_spec *yspec, cxobj *xt); +int clicon_xml_parse_string(char **str, cxobj **xml_top); +#endif +int xml_parse_file(int fd, char *endtag, yang_spec *yspec, cxobj **xt); +int xml_parse_string(const char *str, yang_spec *yspec, cxobj **xml_top); +int xml_parse_va(cxobj **xt, yang_spec *yspec, const char *format, ...); int xmltree2cbuf(cbuf *cb, cxobj *x, int level); int xml_copy_one(cxobj *xn0, cxobj *xn1); diff --git a/lib/src/clixon_options.c b/lib/src/clixon_options.c index fb912b3f..c79ab5a8 100644 --- a/lib/src/clixon_options.c +++ b/lib/src/clixon_options.c @@ -150,7 +150,7 @@ clicon_option_readfile_xml(clicon_hash_t *copt, } clicon_debug(2, "Reading config file %s", __FUNCTION__, filename); fd = fileno(f); - if (clicon_xml_parse_file(fd, "", yspec, &xt) < 0) + if (xml_parse_file(fd, "", yspec, &xt) < 0) goto done; if (xml_child_nr(xt)==1 && xml_child_nr_type(xt, CX_BODY)==1){ clicon_err(OE_CFG, 0, "Config file %s: Expected XML but is probably old sh style", filename); diff --git a/lib/src/clixon_proto.c b/lib/src/clixon_proto.c index 2963c323..93cbaa81 100644 --- a/lib/src/clixon_proto.c +++ b/lib/src/clixon_proto.c @@ -173,7 +173,7 @@ clicon_msg_decode(struct clicon_msg *msg, /* body */ xmlstr = msg->op_body; clicon_debug(1, "%s %s", __FUNCTION__, xmlstr); - if (clicon_xml_parse_str(xmlstr, NULL, xml) < 0) + if (xml_parse_string(xmlstr, NULL, xml) < 0) goto done; retval = 0; done: diff --git a/lib/src/clixon_proto_client.c b/lib/src/clixon_proto_client.c index fb3b1d58..01bf82ad 100644 --- a/lib/src/clixon_proto_client.c +++ b/lib/src/clixon_proto_client.c @@ -120,7 +120,7 @@ clicon_rpc_msg(clicon_handle h, } clicon_debug(1, "%s retdata:%s", __FUNCTION__, retdata); if (retdata && - clicon_xml_parse_str(retdata, NULL, &xret) < 0) + xml_parse_string(retdata, NULL, &xret) < 0) goto done; if (xret0){ *xret0 = xret; diff --git a/lib/src/clixon_xml.c b/lib/src/clixon_xml.c index 515f64ae..f77a11f3 100644 --- a/lib/src/clixon_xml.c +++ b/lib/src/clixon_xml.c @@ -68,8 +68,12 @@ /* * Constants */ -#define BUFLEN 1024 /* Size of xml read buffer */ -#define XML_INDENT 3 /* maybe we should set this programmatically? */ +/* Size of xml read buffer */ +#define BUFLEN 1024 +/* Indentation for xml pretty-print. Consider option? */ +#define XML_INDENT 3 +/* Name of xml top object created by xml parse functions */ +#define XML_TOP_SYMBOL "top" /* * Types @@ -923,6 +927,10 @@ xml_free(cxobj *x) return 0; } +/*------------------------------------------------------------------------ + * XML printing functions. Output a parse tree to file, string cligen buf + *------------------------------------------------------------------------*/ + /*! Print an XML tree structure to an output stream * * Uses clicon_xml2cbuf internally @@ -1130,44 +1138,6 @@ clicon_xml2cbuf(cbuf *cb, done: return retval; } - -/*! Basic xml parsing function. - * @param[in] str Pointer to string containing XML definition. - * @param[in] yspec Yang specification or NULL - * @param[out] xtop Top of XML parse tree. Assume created. - * @see clicon_xml_parse_file clicon_xml_parse_string - */ -int -xml_parse(char *str, - yang_spec *yspec, - cxobj *xt) -{ - int retval = -1; - struct xml_parse_yacc_arg ya = {0,}; - - if (xt == NULL){ - clicon_err(OE_XML, errno, "Unexpected NULL XML"); - return -1; - } - if ((ya.ya_parse_string = strdup(str)) == NULL){ - clicon_err(OE_XML, errno, "strdup"); - return -1; - } - ya.ya_xparent = xt; - ya.ya_skipspace = 1; /* remove all non-terminal bodies (strip pretty-print) */ - ya.ya_yspec = yspec; - if (clixon_xml_parsel_init(&ya) < 0) - goto done; - if (clixon_xml_parseparse(&ya) != 0) /* yacc returns 1 on error */ - goto done; - retval = 0; - done: - clixon_xml_parsel_exit(&ya); - if(ya.ya_parse_string != NULL) - free(ya.ya_parse_string); - return retval; -} - /*! Print actual xml tree datastructures (not xml), mainly for debugging * @param[in,out] cb Cligen buffer to write to * @param[in] xn Clicon xml tree @@ -1207,7 +1177,51 @@ xmltree2cbuf(cbuf *cb, return 0; } -/*! FSM to detect a substring +/*-------------------------------------------------------------------- + * XML parsing functions. Create XML parse tree from string and file. + *--------------------------------------------------------------------*/ +/*! Common internal xml parsing function string to parse-tree + * + * Given a string containing XML, parse into existing XML tree and return + * @param[in] str Pointer to string containing XML definition. + * @param[in] yspec Yang specification or NULL + * @param[in,out] xtop Top of XML parse tree. Assume created. Holds new tree. + * @see xml_parse_file + * @see xml_parse_string + * @see xml_parse_va + */ +static int +xml_parse(const char *str, + yang_spec *yspec, + cxobj *xt) +{ + int retval = -1; + struct xml_parse_yacc_arg ya = {0,}; + + if (xt == NULL){ + clicon_err(OE_XML, errno, "Unexpected NULL XML"); + return -1; + } + if ((ya.ya_parse_string = strdup(str)) == NULL){ + clicon_err(OE_XML, errno, "strdup"); + return -1; + } + ya.ya_xparent = xt; + ya.ya_skipspace = 1; /* remove all non-terminal bodies (strip pretty-print) */ + ya.ya_yspec = yspec; + if (clixon_xml_parsel_init(&ya) < 0) + goto done; + if (clixon_xml_parseparse(&ya) != 0) /* yacc returns 1 on error */ + goto done; + retval = 0; + done: + clixon_xml_parsel_exit(&ya); + if(ya.ya_parse_string != NULL) + free(ya.ya_parse_string); + return retval; +} + +/*! FSM to detect substring */ static inline int FSM(char *tag, @@ -1223,31 +1237,27 @@ FSM(char *tag, /*! Read an XML definition from file and parse it into a parse-tree. * * @param[in] fd A file descriptor containing the XML file (as ASCII characters) - * @param[out] xt Pointer to an (on entry empty) pointer to an XML parse tree - * _created_ by this function. - * @param[in] endtag Read until you encounter "endtag" in the stream, or NULL + * @param[in] endtag Read until encounter "endtag" in the stream, or NULL * @param[in] yspec Yang specification, or NULL - * @retval 0 OK - * @retval -1 Error with clicon_err called + * @param[in,out] xt Pointer to XML parse tree. If empty, create. + * @retval 0 OK + * @retval -1 Error with clicon_err called * * @code * cxobj *xt = NULL; - * clicon_xml_parse_file(0, &xt, ""); + * xml_parse_file(0, "", yspec, &xt); * xml_free(xt); * @endcode - * * @see clicon_xml_parse_str - * Note, you need to free the xml parse tree after use, using xml_free() - * Note, xt will add a top-level symbol called "top" meaning that will look as: - * - * XXX: There is a potential leak here on some return values. - * XXX: What happens if endtag is different? - * May block + * @see xml_parse_string + * @see xml_parse_va + * @note, If xt empty, a top-level symbol will be added so that will be: + * @note May block on file I/O */ int -clicon_xml_parse_file(int fd, - char *endtag, - yang_spec *yspec, - cxobj **xt) +xml_parse_file(int fd, + char *endtag, + yang_spec *yspec, + cxobj **xt) { int retval = -1; int ret; @@ -1255,19 +1265,19 @@ clicon_xml_parse_file(int fd, char ch; char *xmlbuf = NULL; char *ptr; - int maxbuf = BUFLEN; + int xmlbuflen = BUFLEN; /* start size */ int endtaglen = 0; int state = 0; - int oldmaxbuf; + int oldxmlbuflen; if (endtag != NULL) endtaglen = strlen(endtag); *xt = NULL; - if ((xmlbuf = malloc(maxbuf)) == NULL){ + if ((xmlbuf = malloc(xmlbuflen)) == NULL){ clicon_err(OE_XML, errno, "%s: malloc", __FUNCTION__); goto done; } - memset(xmlbuf, 0, maxbuf); + memset(xmlbuf, 0, xmlbuflen); ptr = xmlbuf; while (1){ if ((ret = read(fd, &ch, 1)) < 0){ @@ -1284,20 +1294,21 @@ clicon_xml_parse_file(int fd, if (ret == 0 || (endtag && (state == endtaglen))){ state = 0; - if ((*xt = xml_new("top", NULL, NULL)) == NULL) - break; + if (*xt == NULL) + if ((*xt = xml_new(XML_TOP_SYMBOL, NULL, NULL)) == NULL) + goto done; if (xml_parse(ptr, yspec, *xt) < 0) goto done; break; } - if (len>=maxbuf-1){ /* Space: one for the null character */ - oldmaxbuf = maxbuf; - maxbuf *= 2; - if ((xmlbuf = realloc(xmlbuf, maxbuf)) == NULL){ + if (len>=xmlbuflen-1){ /* Space: one for the null character */ + oldxmlbuflen = xmlbuflen; + xmlbuflen *= 2; + if ((xmlbuf = realloc(xmlbuf, xmlbuflen)) == NULL){ clicon_err(OE_XML, errno, "%s: realloc", __FUNCTION__); goto done; } - memset(xmlbuf+oldmaxbuf, 0, maxbuf-oldmaxbuf); + memset(xmlbuf+oldxmlbuflen, 0, xmlbuflen-oldxmlbuflen); ptr = xmlbuf; } } /* while */ @@ -1314,56 +1325,58 @@ clicon_xml_parse_file(int fd, /*! Read an XML definition from string and parse it into a parse-tree. * - * @param[in] str Pointer to string containing XML definition. - * @param[out] xml_top Top of XML parse tree. Will add extra top element called 'top'. - * you must free it after use, using xml_free() - * @retval 0 OK - * @retval -1 Error with clicon_err called + * @param[in] str String containing XML definition. + * @param[in] yspec Yang specification, or NULL + * @param[in,out] xt Pointer to XML parse tree. If empty will be created. + * @retval 0 OK + * @retval -1 Error with clicon_err called * * @code - * cxobj *cx = NULL; - * if (clicon_xml_parse_str(str, &cx) < 0) + * cxobj *xt = NULL; + * if (xml_parse_string(str, yspec, &xt) < 0) * err; - * xml_free(cx); + * xml_free(xt); * @endcode - * @see clicon_xml_parse_file + * @see xml_parse_file + * @see xml_parse_va * @note you need to free the xml parse tree after use, using xml_free() */ int -clicon_xml_parse_str(char *str, - yang_spec *yspec, - cxobj **cxtop) +xml_parse_string(const char *str, + yang_spec *yspec, + cxobj **xtop) { - if ((*cxtop = xml_new("top", NULL, NULL)) == NULL) - return -1; - return xml_parse(str, yspec, *cxtop); + if (*xtop == NULL) + if ((*xtop = xml_new(XML_TOP_SYMBOL, NULL, NULL)) == NULL) + return -1; + return xml_parse(str, yspec, *xtop); } - -/*! Read XML definition from variable argument string and parse it into parse-tree. +/*! Read XML from var-arg list and parse it into xml tree * * Utility function using stdarg instead of static string. - * @param[out] xml_top Top of XML parse tree. Will add extra top element called 'top'. - * you must free it after use, using xml_free() - * @param[in] yspec Yang specification, or NULL - * @param[in] format Pointer to string containing XML definition. + * @param[in,out] xtop Top of XML parse tree. If it is NULL, top element + called 'top' will be created. Call xml_free() after use + * @param[in] yspec Yang specification, or NULL + * @param[in] format Format string for stdarg according to printf(3) * @retval 0 OK * @retval -1 Error with clicon_err called * * @code - * cxobj *cx = NULL; - * if (clicon_xml_parse(&cx, "%d", 22) < 0) + * cxobj *xt = NULL; + * if (xml_parse_va(&xt, NULL, "%d", 22) < 0) * err; - * xml_free(cx); + * xml_free(xt); * @endcode - * @see clicon_xml_parse_str - * @note you need to free the xml parse tree after use, using xml_free() + * @see xml_parse_string + * @see xml_parse_file + * @note If vararg list is empty, consider using xml_parse_string() */ int -clicon_xml_parse(cxobj **cxtop, - yang_spec *yspec, - char *format, ...) +xml_parse_va(cxobj **xtop, + yang_spec *yspec, + const char *format, ...) { int retval = -1; va_list args; @@ -1381,9 +1394,10 @@ clicon_xml_parse(cxobj **cxtop, va_start(args, format); len = vsnprintf(str, len, format, args) + 1; va_end(args); - if ((*cxtop = xml_new("top", NULL, NULL)) == NULL) - return -1; - if (xml_parse(str, yspec, *cxtop) < 0) + if (*xtop == NULL) + if ((*xtop = xml_new(XML_TOP_SYMBOL, NULL, NULL)) == NULL) + goto done; + if (xml_parse(str, yspec, *xtop) < 0) goto done; retval = 0; done: @@ -2103,7 +2117,7 @@ main(int argc, char **argv) usage(argv[0]); return 0; } - if (clicon_xml_parse_file(0, &xt, "") < 0){ + if (xml_parse_file(0, "", NULL,&xt) < 0){ fprintf(stderr, "parsing 2\n"); return -1; } diff --git a/lib/src/clixon_xml_db.c b/lib/src/clixon_xml_db.c index 6ff4c90a..c07be468 100644 --- a/lib/src/clixon_xml_db.c +++ b/lib/src/clixon_xml_db.c @@ -382,7 +382,7 @@ xmldb_get(clicon_handle h, * The xml may contain the "operation" attribute which defines the operation. * @code * cxobj *xt; - * if (clicon_xml_parse_str("17", &xt) < 0) + * if (xml_parse_string("17", yspec, &xt) < 0) * err; * if (xmldb_put(xh, "running", OP_MERGE, xt) < 0) * err; diff --git a/lib/src/clixon_xsl.c b/lib/src/clixon_xsl.c index 2a67de00..e33cecc1 100644 --- a/lib/src/clixon_xsl.c +++ b/lib/src/clixon_xsl.c @@ -1174,7 +1174,7 @@ main(int argc, char **argv) usage(argv[0]); return 0; } - if (clicon_xml_parse_file(0, &x, "") < 0){ + if (xml_parse_file(0, &x, "") < 0){ fprintf(stderr, "parsing 2\n"); return -1; } diff --git a/lib/src/json_xpath.c b/lib/src/json_xpath.c index d0574f4c..5ac6d152 100644 --- a/lib/src/json_xpath.c +++ b/lib/src/json_xpath.c @@ -98,7 +98,7 @@ main(int argc, char **argv) return -1; } else - if (clicon_xml_parse_str(buf, &x) < 0) + if (xml_parse_string(buf, &x) < 0) return -1; if (xpath_vec(x, xpath, &xv, &xlen) < 0) diff --git a/test/test_cli.sh b/test/test_cli.sh index c5d224f5..c208d4de 100755 --- a/test/test_cli.sh +++ b/test/test_cli.sh @@ -9,6 +9,7 @@ # include err() and new() functions . ./lib.sh +clixon_cf="-f /usr/local/etc/routing.xml" # For memcheck #clixon_cli="valgrind --leak-check=full --show-leak-kinds=all clixon_cli" @@ -16,83 +17,81 @@ clixon_cli=clixon_cli # kill old backend (if any) new "kill old backend" -sudo clixon_backend -zf $clixon_cf +sudo clixon_backend -z $clixon_cf if [ $? -ne 0 ]; then err fi new "start backend" # start new backend -sudo clixon_backend -s init -f $clixon_cf +sudo clixon_backend -s init $clixon_cf if [ $? -ne 0 ]; then err fi new "cli tests" new "cli configure top" -expectfn "$clixon_cli -1f $clixon_cf set interfaces" "^$" +expectfn "$clixon_cli -1 $clixon_cf set interfaces" "^$" new "cli show configuration top (no presence)" -expectfn "$clixon_cli -1f $clixon_cf show conf cli" "^$" +expectfn "$clixon_cli -1 $clixon_cf show conf cli" "^$" new "cli configure delete top" -expectfn "$clixon_cli -1f $clixon_cf delete interfaces" "^$" +expectfn "$clixon_cli -1 $clixon_cf delete interfaces" "^$" new "cli show configuration delete top" -expectfn "$clixon_cli -1f $clixon_cf show conf cli" "^$" +expectfn "$clixon_cli -1 $clixon_cf show conf cli" "^$" new "cli configure" -expectfn "$clixon_cli -1f $clixon_cf set interfaces interface eth/0/0" "^$" +expectfn "$clixon_cli -1 $clixon_cf set interfaces interface eth/0/0" "^$" new "cli show configuration" -expectfn "$clixon_cli -1f $clixon_cf show conf cli" "^interfaces interface name eth/0/0" "interfaces interface enabled true$" +expectfn "$clixon_cli -1 $clixon_cf show conf cli" "^interfaces interface name eth/0/0" "interfaces interface enabled true$" new "cli failed validate" -expectfn "$clixon_cli -1f $clixon_cf -l o validate" "Missing mandatory variable" +expectfn "$clixon_cli -1 $clixon_cf -l o validate" "Missing mandatory variable" new "cli configure more" -expectfn "$clixon_cli -1f $clixon_cf set interfaces interface eth/0/0 ipv4 address 1.2.3.4 prefix-length 24" "^$" -expectfn "$clixon_cli -1f $clixon_cf set interfaces interface eth/0/0 description mydesc" "^$" -expectfn "$clixon_cli -1f $clixon_cf set interfaces interface eth/0/0 type bgp" "^$" +expectfn "$clixon_cli -1 $clixon_cf set interfaces interface eth/0/0 ipv4 address 1.2.3.4 prefix-length 24" "^$" +expectfn "$clixon_cli -1 $clixon_cf set interfaces interface eth/0/0 description mydesc" "^$" +expectfn "$clixon_cli -1 $clixon_cf set interfaces interface eth/0/0 type bgp" "^$" new "cli show xpath description" -expectfn "$clixon_cli -1f $clixon_cf -l o show xpath /interfaces/interface/description" "mydesc" +expectfn "$clixon_cli -1 $clixon_cf -l o show xpath /interfaces/interface/description" "mydesc" new "cli delete description" -expectfn "$clixon_cli -1f $clixon_cf -l o delete interfaces interface eth/0/0 description mydesc" +expectfn "$clixon_cli -1 $clixon_cf -l o delete interfaces interface eth/0/0 description mydesc" new "cli show xpath no description" -expectfn "$clixon_cli -1f $clixon_cf -l o show xpath /interfaces/interface/description" "^$" +expectfn "$clixon_cli -1 $clixon_cf -l o show xpath /interfaces/interface/description" "^$" new "cli copy interface" -expectfn "$clixon_cli -1f $clixon_cf copy interface eth/0/0 to eth99" "^$" +expectfn "$clixon_cli -1 $clixon_cf copy interface eth/0/0 to eth99" "^$" new "cli success validate" -expectfn "$clixon_cli -1f $clixon_cf -l o validate" "^$" +expectfn "$clixon_cli -1 $clixon_cf -l o validate" "^$" new "cli commit" -expectfn "$clixon_cli -1f $clixon_cf -l o commit" "^$" +expectfn "$clixon_cli -1 $clixon_cf -l o commit" "^$" new "cli save" -expectfn "$clixon_cli -1f $clixon_cf -l o save /tmp/foo" "^$" - - +expectfn "$clixon_cli -1 $clixon_cf -l o save /tmp/foo" "^$" new "cli delete all" -expectfn "$clixon_cli -1f $clixon_cf -l o delete all" "^$" +expectfn "$clixon_cli -1 $clixon_cf -l o delete all" "^$" new "cli load" -expectfn "$clixon_cli -1f $clixon_cf -l o load /tmp/foo" "^$" +expectfn "$clixon_cli -1 $clixon_cf -l o load /tmp/foo" "^$" new "cli check load" -expectfn "$clixon_cli -1f $clixon_cf -l o show conf cli" "^interfaces interface name eth/0/0" "interfaces interface enabled true$" +expectfn "$clixon_cli -1 $clixon_cf -l o show conf cli" "^interfaces interface name eth/0/0" "interfaces interface enabled true$" new "cli debug" -expectfn "$clixon_cli -1f $clixon_cf -l o debug level 1" "^$" +expectfn "$clixon_cli -1 $clixon_cf -l o debug level 1" "^$" # How to test this? -expectfn "$clixon_cli -1f $clixon_cf -l o debug level 0" "^$" +expectfn "$clixon_cli -1 $clixon_cf -l o debug level 0" "^$" new "cli rpc" -expectfn "$clixon_cli -1f $clixon_cf -l o rpc ipv4" "^" +expectfn "$clixon_cli -1 $clixon_cf -l o rpc ipv4" "^" new "Kill backend" # Check if still alive @@ -101,7 +100,7 @@ if [ -z "$pid" ]; then err "backend already dead" fi # kill backend -sudo clixon_backend -zf $clixon_cf +sudo clixon_backend -z $clixon_cf if [ $? -ne 0 ]; then err "kill backend" fi