From 45d8e5b6ce2cf2d7873f0279945e4aed78d3d648 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Sun, 9 Apr 2023 17:08:53 +0200 Subject: [PATCH] C-API `xmldb_validate()` removed. `compare_db_names()` added. --- CHANGELOG.md | 1 + apps/backend/backend_client.c | 36 ------------------- apps/backend/backend_commit.c | 2 -- apps/cli/cli_common.c | 66 +++++++++++++++++++++++------------ apps/cli/clixon_cli_api.h | 2 ++ lib/clixon/clixon_datastore.h | 1 - lib/src/clixon_datastore.c | 17 --------- lib/src/clixon_proto.c | 4 ++- lib/src/clixon_xpath.c | 6 ++-- 9 files changed, 54 insertions(+), 81 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fb4a9ec..bd205ef9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,7 @@ Users may have to change how they access the system Developers may need to change their code * C-API + * `xmldb_validate` is removed. Yang checks should be enough, remnant of time before YANG checks. * `xml_diff`: removed 1st `yspec` parameter * `xml2xpath()`: Added `int apostrophe` as 4th parameter, default 0 * This is for being able to choose single or double quote as xpath literal quotes diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index 4de0b6ed..9c9893a1 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -446,12 +446,6 @@ from_client_edit_config(clicon_handle h, clicon_err(OE_XML, errno, "cbuf_new"); goto done; } - if (xmldb_validate_db(target) < 0){ - cprintf(cbx, "No such database: %s", target); - if (netconf_invalid_value(cbret, "protocol", cbuf_get(cbx))< 0) - goto done; - goto ok; - } /* Check if target locked by other client */ iddb = xmldb_islocked(h, target); if (iddb && myid != iddb){ @@ -660,23 +654,11 @@ from_client_copy_config(clicon_handle h, clicon_err(OE_XML, errno, "cbuf_new"); goto done; } - if (xmldb_validate_db(source) < 0){ - cprintf(cbx, "No such database: %s", source); - if (netconf_invalid_value(cbret, "protocol", cbuf_get(cbx))< 0) - goto done; - goto ok; - } if ((target = netconf_db_find(xe, "target")) == NULL){ if (netconf_missing_element(cbret, "protocol", "target", NULL) < 0) goto done; goto ok; } - if (xmldb_validate_db(target) < 0){ - cprintf(cbx, "No such database: %s", target); - if (netconf_invalid_value(cbret, "protocol", cbuf_get(cbx))< 0) - goto done; - goto ok; - } /* Check if target locked by other client */ iddb = xmldb_islocked(h, target); if (iddb && myid != iddb){ @@ -743,12 +725,6 @@ from_client_delete_config(clicon_handle h, clicon_err(OE_XML, errno, "cbuf_new"); goto done; } - if (xmldb_validate_db(target) < 0){ - cprintf(cbx, "No such database: %s", target); - if (netconf_invalid_value(cbret, "protocol", cbuf_get(cbx))< 0) - goto done; - goto ok; - } /* Check if target locked by other client */ iddb = xmldb_islocked(h, target); if (iddb && myid != iddb){ @@ -828,12 +804,6 @@ from_client_lock(clicon_handle h, clicon_err(OE_XML, errno, "cbuf_new"); goto done; } - if (xmldb_validate_db(db) < 0){ - cprintf(cbx, "No such database: %s", db); - if (netconf_invalid_value(cbret, "protocol", cbuf_get(cbx))< 0) - goto done; - goto ok; - } /* * A lock MUST not be granted if either of the following conditions is true: * 1) A lock is already held by any NETCONF session or another entity. @@ -915,12 +885,6 @@ from_client_unlock(clicon_handle h, clicon_err(OE_XML, errno, "cbuf_new"); goto done; } - if (xmldb_validate_db(db) < 0){ - cprintf(cbx, "No such database: %s", db); - if (netconf_invalid_value(cbret, "protocol", cbuf_get(cbx))< 0) - goto done; - goto ok; - } iddb = xmldb_islocked(h, db); /* * An unlock operation will not succeed if any of the following diff --git a/apps/backend/backend_commit.c b/apps/backend/backend_commit.c index fd83aefd..e642a6ec 100644 --- a/apps/backend/backend_commit.c +++ b/apps/backend/backend_commit.c @@ -802,14 +802,12 @@ from_client_commit(clicon_handle h, clicon_err(OE_YANG, ENOENT, "No yang spec"); goto done; } - if (if_feature(yspec, "ietf-netconf", "confirmed-commit")) { if ((ret = from_client_confirmed_commit(h, xe, myid, cbret)) < 0) goto done; if (ret == 0) goto ok; } - /* Check if target locked by other client */ iddb = xmldb_islocked(h, "running"); if (iddb && myid != iddb){ diff --git a/apps/cli/cli_common.c b/apps/cli/cli_common.c index eab11f79..d04f8578 100644 --- a/apps/cli/cli_common.c +++ b/apps/cli/cli_common.c @@ -835,37 +835,31 @@ cli_validate(clicon_handle h, return retval; } -/*! Compare two dbs using XML. Write to file and run diff - * @param[in] h Clicon handle - * @param[in] cvv - * @param[in] argv arg: 0 as xml, 1: as text +/*! Compare two datastore by name and formats + * + * @param[in] h Clixon handle + * @param[in] format Output format + * @param[in] db1 Name of first datastrore + * @param[in] db2 Name of second datastrore */ int -compare_dbs(clicon_handle h, - cvec *cvv, - cvec *argv) +compare_db_names(clicon_handle h, + enum format_enum format, + char *db1, + char *db2) { - cxobj *xc1 = NULL; /* running xml */ - cxobj *xc2 = NULL; /* candidate xml */ - cxobj *xerr = NULL; - int retval = -1; - enum format_enum format; + int retval = -1; + cxobj *xc1 = NULL; + cxobj *xc2 = NULL; + cxobj *xerr = NULL; - if (cvec_len(argv) > 1){ - clicon_err(OE_PLUGIN, EINVAL, "Requires 0 or 1 element. If given: astext flag 0|1"); - goto done; - } - if (cvec_len(argv) && cv_int32_get(cvec_i(argv, 0)) == 1) - format = FORMAT_TEXT; - else - format = FORMAT_XML; - if (clicon_rpc_get_config(h, NULL, "running", "/", NULL, NULL, &xc1) < 0) + if (clicon_rpc_get_config(h, NULL, db1, "/", NULL, NULL, &xc1) < 0) goto done; if ((xerr = xpath_first(xc1, NULL, "/rpc-error")) != NULL){ clixon_netconf_error(xerr, "Get configuration", NULL); goto done; } - if (clicon_rpc_get_config(h, NULL, "candidate", "/", NULL, NULL, &xc2) < 0) + if (clicon_rpc_get_config(h, NULL, db2, "/", NULL, NULL, &xc2) < 0) goto done; if ((xerr = xpath_first(xc2, NULL, "/rpc-error")) != NULL){ clixon_netconf_error(xerr, "Get configuration", NULL); @@ -882,6 +876,34 @@ compare_dbs(clicon_handle h, return retval; } +/*! Compare two dbs using XML. Write to file and run diff + * @param[in] h Clicon handle + * @param[in] cvv + * @param[in] argv arg: 0 as xml, 1: as text + */ +int +compare_dbs(clicon_handle h, + cvec *cvv, + cvec *argv) +{ + int retval = -1; + enum format_enum format; + + if (cvec_len(argv) > 1){ + clicon_err(OE_PLUGIN, EINVAL, "Requires 0 or 1 element. If given: astext flag 0|1"); + goto done; + } + if (cvec_len(argv) && cv_int32_get(cvec_i(argv, 0)) == 1) + format = FORMAT_TEXT; + else + format = FORMAT_XML; + if (compare_db_names(h, format, "running", "candidate") < 0) + goto done; + retval = 0; + done: + return retval; +} + /*! Load a configuration file to candidate database * Utility function used by cligen spec file * Note that the CLI function makes no Validation of the XML sent to the backend diff --git a/apps/cli/clixon_cli_api.h b/apps/cli/clixon_cli_api.h index 5e498dd4..eda9e565 100644 --- a/apps/cli/clixon_cli_api.h +++ b/apps/cli/clixon_cli_api.h @@ -112,6 +112,8 @@ int cli_commit(clicon_handle h, cvec *vars, cvec *argv); int cli_validate(clicon_handle h, cvec *vars, cvec *argv); +int compare_db_names(clicon_handle h, enum format_enum format, char *db1, char *db2); + int compare_dbs(clicon_handle h, cvec *vars, cvec *argv); int load_config_file(clicon_handle h, cvec *vars, cvec *argv); diff --git a/lib/clixon/clixon_datastore.h b/lib/clixon/clixon_datastore.h index bb843364..4ccf8892 100644 --- a/lib/clixon/clixon_datastore.h +++ b/lib/clixon/clixon_datastore.h @@ -46,7 +46,6 @@ int xmldb_db2file(clicon_handle h, const char *db, char **filename); /* API */ -int xmldb_validate_db(const char *db); int xmldb_connect(clicon_handle h); int xmldb_disconnect(clicon_handle h); /* in clixon_datastore_read.[ch] */ diff --git a/lib/src/clixon_datastore.c b/lib/src/clixon_datastore.c index d1d85a0d..4202dbbb 100644 --- a/lib/src/clixon_datastore.c +++ b/lib/src/clixon_datastore.c @@ -121,23 +121,6 @@ xmldb_db2file(clicon_handle h, return retval; } -/*! Ensure database name is correct - * @param[in] db Name of database - * @retval 0 OK - * @retval -1 Failed validate, xret set to error - * XXX why is this function here? should be handled by netconf yang validation - */ -int -xmldb_validate_db(const char *db) -{ - if (strcmp(db, "running") != 0 && - strcmp(db, "candidate") != 0 && - strcmp(db, "startup") != 0 && - strcmp(db, "tmp") != 0) - return -1; - return 0; -} - /*! Connect to a datastore plugin, allocate resources to be used in API calls * @param[in] h Clicon handle * @retval 0 OK diff --git a/lib/src/clixon_proto.c b/lib/src/clixon_proto.c index a3223b8e..4fc9a7f9 100644 --- a/lib/src/clixon_proto.c +++ b/lib/src/clixon_proto.c @@ -379,6 +379,8 @@ clicon_msg_send(int s, * @param[in] intr If set, make a ^C cause an error * @param[out] msg CLICON msg data reply structure. Free with free() * @param[out] eof Set if eof encountered + * @retval 0 OK + * @retval -1 Error * Note: caller must ensure that s is closed if eof is set after call. * @see clicon_msg_rcv1 using plain NETCONF */ @@ -451,7 +453,7 @@ clicon_msg_rcv(int s, clicon_debug(CLIXON_DBG_MSG, "Recv: %s", (*msg)->op_body); ok: retval = 0; - done: + done: clicon_debug(CLIXON_DBG_DETAIL, "%s retval:%d", __FUNCTION__, retval); if (intr){ set_signal(SIGINT, oldhandler, NULL); diff --git a/lib/src/clixon_xpath.c b/lib/src/clixon_xpath.c index e84103ad..e1811830 100644 --- a/lib/src/clixon_xpath.c +++ b/lib/src/clixon_xpath.c @@ -1264,8 +1264,10 @@ xml2xpath1(cxobj *x, cvi = NULL; while ((cvi = cvec_each(cvk, cvi)) != NULL) { keyname = cv_string_get(cvi); - if ((xkey = xml_find(x, keyname)) == NULL) - goto done; /* No key in xml */ + if ((xkey = xml_find(x, keyname)) == NULL){ + clicon_err(OE_XML, 0, "No key %s in list %s", keyname, xml_name(x)); + goto done; + } if ((xb = xml_find(x, keyname)) == NULL) goto done; b = xml_body(xb);