Skip to content

Commit

Permalink
config_new REFACTOR code review
Browse files Browse the repository at this point in the history
  • Loading branch information
Roytak committed Oct 4, 2023
1 parent 93d3eff commit 94ec228
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 41 deletions.
106 changes: 73 additions & 33 deletions src/config_new.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,13 @@
#include "session_p.h"

int
nc_config_new_delete(struct lyd_node **tree, const char *path_fmt, ...)
nc_config_new_create(const struct ly_ctx *ctx, struct lyd_node **tree, const char *value, const char *path_fmt, ...)
{
int ret = 0;
va_list ap;
char *path = NULL;
struct lyd_node *sub = NULL;

NC_CHECK_ARG_RET(NULL, ctx, tree, path_fmt, 1);

va_start(ap, path_fmt);

Expand All @@ -52,15 +53,18 @@ nc_config_new_delete(struct lyd_node **tree, const char *path_fmt, ...)
goto cleanup;
}

/* find the node we want to delete */
ret = lyd_find_path(*tree, path, 0, &sub);
/* create the nodes in the path */
if (!*tree) {
ret = lyd_new_path(*tree, ctx, path, value, LYD_NEW_PATH_UPDATE, tree);
} else {
/* this could output NULL if no new nodes, lyd_find_path would fail then */
ret = lyd_new_path(*tree, ctx, path, value, LYD_NEW_PATH_UPDATE, NULL);
}
if (ret) {
goto cleanup;
}

lyd_free_tree(sub);

/* set the node to top level container */
/* set the node to the top level node */
ret = lyd_find_path(*tree, "/ietf-netconf-server:netconf-server", 0, tree);
if (ret) {
goto cleanup;
Expand All @@ -79,16 +83,16 @@ nc_config_new_delete(struct lyd_node **tree, const char *path_fmt, ...)
}

int
nc_config_new_create(const struct ly_ctx *ctx, struct lyd_node **tree, const char *value, const char *path_fmt, ...)
nc_config_new_create_append(const struct ly_ctx *ctx, const char *parent_path, const char *child_name,
const char *value, struct lyd_node **tree)
{
int ret = 0;
va_list ap;
char *path = NULL;

va_start(ap, path_fmt);
NC_CHECK_ARG_RET(NULL, ctx, parent_path, child_name, tree, 1);

/* create the path from the format */
ret = vasprintf(&path, path_fmt, ap);
/* create the path by appending child to the parent path */
ret = asprintf(&path, "%s/%s", parent_path, child_name);
if (ret == -1) {
ERRMEM;
path = NULL;
Expand Down Expand Up @@ -120,37 +124,38 @@ nc_config_new_create(const struct ly_ctx *ctx, struct lyd_node **tree, const cha

cleanup:
free(path);
va_end(ap);
return ret;
}

int
nc_config_new_create_append(const struct ly_ctx *ctx, const char *parent_path, const char *child_name,
const char *value, struct lyd_node **tree)
nc_config_new_delete(struct lyd_node **tree, const char *path_fmt, ...)
{
int ret = 0;
va_list ap;
char *path = NULL;
struct lyd_node *sub = NULL;

/* create the path by appending child to the parent path */
ret = asprintf(&path, "%s/%s", parent_path, child_name);
NC_CHECK_ARG_RET(NULL, tree, path_fmt, 1);

va_start(ap, path_fmt);

/* create the path from the format */
ret = vasprintf(&path, path_fmt, ap);
if (ret == -1) {
ERRMEM;
path = NULL;
goto cleanup;
}

/* create the nodes in the path */
if (!*tree) {
ret = lyd_new_path(*tree, ctx, path, value, LYD_NEW_PATH_UPDATE, tree);
} else {
/* this could output NULL if no new nodes, lyd_find_path would fail then */
ret = lyd_new_path(*tree, ctx, path, value, LYD_NEW_PATH_UPDATE, NULL);
}
/* find the node we want to delete */
ret = lyd_find_path(*tree, path, 0, &sub);
if (ret) {
goto cleanup;
}

/* set the node to the top level node */
lyd_free_tree(sub);

/* set the node to top level container */
ret = lyd_find_path(*tree, "/ietf-netconf-server:netconf-server", 0, tree);
if (ret) {
goto cleanup;
Expand All @@ -164,6 +169,7 @@ nc_config_new_create_append(const struct ly_ctx *ctx, const char *parent_path, c

cleanup:
free(path);
va_end(ap);
return ret;
}

Expand All @@ -175,6 +181,8 @@ nc_config_new_check_delete(struct lyd_node **tree, const char *path_fmt, ...)
char *path = NULL;
struct lyd_node *sub = NULL;

NC_CHECK_ARG_RET(NULL, tree, path_fmt, 1);

va_start(ap, path_fmt);

/* create the path from the format */
Expand Down Expand Up @@ -235,6 +243,8 @@ nc_server_config_new_pubkey_bin_to_b64(const unsigned char *pub_bin, int bin_len
int ret = 0, b64_len;
char *pub_b64 = NULL;

NC_CHECK_ARG_RET(NULL, pub_bin, bin_len, pubkey, 1);

/* get b64 buffer len, for ever 3 bytes of bin 4 bytes of b64 + NULL terminator */
if (bin_len % 3 == 0) {
pub_b64 = malloc((bin_len / 3) * 4 + 1);
Expand Down Expand Up @@ -322,6 +332,8 @@ nc_server_config_new_evp_pkey_to_ssh_pubkey(EVP_PKEY *pkey, char **pubkey)
uint32_t alg_name_len, curve_name_len, alg_name_len_be, curve_name_len_be, p_len_be, e_len_be, n_len_be;
size_t ec_group_len;

NC_CHECK_ARG_RET(NULL, pkey, pubkey, 1);

if (EVP_PKEY_is_a(pkey, "RSA")) {
/* RSA key */
algorithm_name = "ssh-rsa";
Expand Down Expand Up @@ -467,6 +479,7 @@ nc_server_config_new_evp_pkey_to_ssh_pubkey(EVP_PKEY *pkey, char **pubkey)
goto cleanup;
}

/* convert created bin to b64 */
ret = nc_server_config_new_pubkey_bin_to_b64(bin, bin_len, pubkey);
if (ret) {
ERR(NULL, "Converting public key from binary to base64 failed.");
Expand All @@ -493,6 +506,8 @@ nc_server_config_new_evp_pkey_to_spki_pubkey(EVP_PKEY *pkey, char **pubkey)
BIO *bio = NULL;
char *pub_b64 = NULL;

NC_CHECK_ARG_RET(NULL, pkey, pubkey, 1);

bio = BIO_new(BIO_s_mem());
if (!bio) {
ERR(NULL, "Creating new BIO failed (%s).", ERR_reason_error_string(ERR_get_error()));
Expand Down Expand Up @@ -538,7 +553,7 @@ nc_server_config_new_read_certificate(const char *cert_path, char **cert)
BIO *bio = NULL;
char *c = NULL;

*cert = NULL;
NC_CHECK_ARG_RET(NULL, cert_path, cert, 1);

f = fopen(cert_path, "r");
if (!f) {
Expand Down Expand Up @@ -623,16 +638,22 @@ nc_server_config_new_read_pubkey_ssh2(FILE *f, char **pubkey)
ssize_t read;
int ret = 0;

NC_CHECK_ARG_RET(NULL, f, pubkey, 1);

/* read lines from the file and create the public key without NL from it */
while ((read = getline(&buffer, &size, f)) > 0) {
if (!strncmp(buffer, "----", 4)) {
/* skip header and footer */
continue;
}

if (!strncmp(buffer, "Comment:", 8)) {
/* skip a comment */
continue;
}

if (buffer[read - 1] == '\n') {
/* avoid NL */
read--;
}

Expand Down Expand Up @@ -803,6 +824,8 @@ nc_server_config_new_privkey_header_to_format(FILE *f_privkey, const char *privk
char *privkey_header = NULL;
size_t len = 0;

NC_CHECK_ARG_RET(NULL, f_privkey, privkey_path, privkey_format, 1);

/* read header */
if (getline(&privkey_header, &len, f_privkey) < 0) {
ERR(NULL, "Error reading header from file \"%s\".", privkey_path);
Expand Down Expand Up @@ -840,6 +863,8 @@ nc_server_config_new_get_privkey_openssl(const char *privkey_path, FILE *f_privk
BIO *bio = NULL;
char *priv_b64 = NULL;

NC_CHECK_ARG_RET(NULL, privkey_path, f_privkey, privkey, pkey, 1);

bio = BIO_new(BIO_s_mem());
if (!bio) {
ERR(NULL, "Creating new BIO failed (%s).", ERR_reason_error_string(ERR_get_error()));
Expand Down Expand Up @@ -871,8 +896,14 @@ nc_server_config_new_get_privkey_openssl(const char *privkey_path, FILE *f_privk
}

*privkey = strndup(priv_b64, len);
if (!*privkey) {
ERRMEM;
ret = 1;
goto cleanup;
}

cleanup:
/* priv_b64 is freed with BIO */
BIO_free(bio);
return ret;
}
Expand All @@ -885,6 +916,8 @@ nc_server_config_new_get_privkey_libssh(const char *privkey_path, char **privkey
char *priv_b64 = NULL;
ssh_key key = NULL;

NC_CHECK_ARG_RET(NULL, privkey_path, privkey, pkey, 1);

ret = ssh_pki_import_privkey_file(privkey_path, NULL, NULL, NULL, &key);
if (ret) {
ERR(NULL, "Importing privkey from file \"%s\" failed.", privkey_path);
Expand Down Expand Up @@ -921,6 +954,11 @@ nc_server_config_new_get_privkey_libssh(const char *privkey_path, char **privkey
}

*privkey = strndup(priv_b64, ret);
if (!*privkey) {
ERRMEM;
ret = 1;
goto cleanup;
}

/* ok */
ret = 0;
Expand All @@ -939,6 +977,8 @@ nc_server_config_new_get_privkey(const char *privkey_path, NC_PRIVKEY_FORMAT *pr
FILE *f_privkey = NULL;
char *priv = NULL;

NC_CHECK_ARG_RET(NULL, privkey_path, privkey_format, privkey, pkey, 1);

f_privkey = fopen(privkey_path, "r");
if (!f_privkey) {
ERR(NULL, "Unable to open file \"%s\".", privkey_path);
Expand Down Expand Up @@ -1049,7 +1089,7 @@ nc_server_config_new_address_port(const struct ly_ctx *ctx, const char *endpt_na
const char *address_fmt, *port_fmt;
char port_buf[6] = {0};

NC_CHECK_ARG_RET(NULL, address, ctx, endpt_name, config, 1);
NC_CHECK_ARG_RET(NULL, ctx, endpt_name, address, config, 1);

if (transport == NC_TI_LIBSSH) {
/* SSH path */
Expand All @@ -1060,7 +1100,7 @@ nc_server_config_new_address_port(const struct ly_ctx *ctx, const char *endpt_na
address_fmt = "/ietf-netconf-server:netconf-server/listen/endpoint[name='%s']/tls/tcp-server-parameters/local-address";
port_fmt = "/ietf-netconf-server:netconf-server/listen/endpoint[name='%s']/tls/tcp-server-parameters/local-port";
} else {
ERR(NULL, "Transport not supported.");
ERR(NULL, "Can not set address and port of a non SSH/TLS endpoint.");
ret = 1;
goto cleanup;
}
Expand All @@ -1087,7 +1127,7 @@ nc_server_config_new_ch_address_port(const struct ly_ctx *ctx, const char *clien
int ret = 0;
const char *address_fmt, *port_fmt;

NC_CHECK_ARG_RET(NULL, address, port, ctx, endpt_name, config, 1);
NC_CHECK_ARG_RET(NULL, ctx, client_name, endpt_name, address, port, config, 1);

if (transport == NC_TI_LIBSSH) {
/* SSH path */
Expand Down Expand Up @@ -1130,7 +1170,7 @@ nc_server_config_new_del_endpt(const char *endpt_name, struct lyd_node **config)
}

API int
nc_server_config_new_del_ch_client(const char *ch_client_name, struct lyd_node **config)
nc_server_config_new_ch_del_ch_client(const char *ch_client_name, struct lyd_node **config)
{
NC_CHECK_ARG_RET(NULL, config, 1);

Expand Down Expand Up @@ -1447,7 +1487,7 @@ nc_server_config_new_ch_period(const struct ly_ctx *ctx, const char *ch_client_n
{
char buf[6] = {0};

NC_CHECK_ARG_RET(NULL, ctx, ch_client_name, period, 1);
NC_CHECK_ARG_RET(NULL, ctx, ch_client_name, config, 1);

/* delete persistent tree if exists */
if (nc_config_new_check_delete(config, "/ietf-netconf-server:netconf-server/call-home/"
Expand All @@ -1473,7 +1513,7 @@ API int
nc_server_config_new_ch_anchor_time(const struct ly_ctx *ctx, const char *ch_client_name,
const char *anchor_time, struct lyd_node **config)
{
NC_CHECK_ARG_RET(NULL, ctx, ch_client_name, anchor_time, 1);
NC_CHECK_ARG_RET(NULL, ctx, ch_client_name, anchor_time, config, 1);

/* delete persistent tree if exists */
if (nc_config_new_check_delete(config, "/ietf-netconf-server:netconf-server/call-home/"
Expand All @@ -1500,7 +1540,7 @@ nc_server_config_new_ch_idle_timeout(const struct ly_ctx *ctx, const char *ch_cl
{
char buf[6] = {0};

NC_CHECK_ARG_RET(NULL, ctx, ch_client_name, 1);
NC_CHECK_ARG_RET(NULL, ctx, ch_client_name, config, 1);

/* delete persistent tree if exists */
if (nc_config_new_check_delete(config, "/ietf-netconf-server:netconf-server/call-home/"
Expand Down
18 changes: 13 additions & 5 deletions src/config_new.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ const char * nc_config_new_privkey_format_to_identityref(NC_PRIVKEY_FORMAT forma
*
* @param[in] ctx libyang context.
* @param[in, out] tree The YANG data tree where the insertion will happen. On success
* the top level container is always returned.
* this is set to the top level container.
* @param[in] value Value assigned to the final node in the path.
* @param[in] path_fmt Format of the path.
* @param[in] ... Parameters for the path format, essentially representing the lists' keys.
Expand All @@ -98,14 +98,14 @@ const char * nc_config_new_privkey_format_to_identityref(NC_PRIVKEY_FORMAT forma
int nc_config_new_create(const struct ly_ctx *ctx, struct lyd_node **tree, const char *value, const char *path_fmt, ...);

/**
* @brief Creates new YANG data nodes in a path and gives the final node a value.
* @brief Creates a YANG data node by appending it to a specified parent node.
*
* @param[in] ctx libyang context.
* @param[in] parent_path Path to the parent node.
* @param[in] child_name Name of the parent's child node to be created.
* @param[in] value Value to give to the child node.
* @param[in] value Value given to the child node.
* @param[out] tree YANG data tree where the insertion will happen. On success
* the top level container is always returned.
* this is set to the top level container.
* @return 0 on success, 1 otherwise.
*/
int nc_config_new_create_append(const struct ly_ctx *ctx, const char *parent_path, const char *child_name,
Expand All @@ -115,12 +115,20 @@ int nc_config_new_create_append(const struct ly_ctx *ctx, const char *parent_pat
* @brief Deletes a subtree from the YANG data.
*
* @param tree YANG data from which the subtree will be deleted.
* @param[in] path_fmt Format of the path
* @param[in] path_fmt Format of the path. The last node will be the top level node of the deleted tree.
* @param[in] ... Parameters for the path format, essentially representing the lists' keys.
* @return 0 on success, non-zero otherwise.
*/
int nc_config_new_delete(struct lyd_node **tree, const char *path_fmt, ...);

/**
* @brief Deletes a subtree from the YANG data, but doesn't return an error if the node doesn't exist.
*
* @param tree YANG data from which the subtree will be deleted.
* @param[in] path_fmt Format of the path. The last node will be the top level node of the deleted tree.
* @param[in] ... Parameters for the path format, essentially representing the lists' keys.
* @return 0 on success, non-zero otherwise.
*/
int nc_config_new_check_delete(struct lyd_node **tree, const char *path_fmt, ...);

#ifdef __cplusplus
Expand Down
2 changes: 1 addition & 1 deletion src/server_config.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1056,7 +1056,7 @@ int nc_server_config_new_ch_address_port(const struct ly_ctx *ctx, const char *c
* @param[in,out] config Modified configuration YANG data tree.
* @return 0 on success, non-zero otherwise.
*/
int nc_server_config_new_del_ch_client(const char *client_name, struct lyd_node **config);
int nc_server_config_new_ch_del_ch_client(const char *client_name, struct lyd_node **config);

/**
* @brief Deletes a Call Home endpoint from the YANG data.
Expand Down
4 changes: 2 additions & 2 deletions tests/test_ch.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ server_thread_ssh(void *arg)
strcpy(expected, "reconnecting in");

/* prepare data for deleting the call-home client */
ret = nc_server_config_new_del_ch_client("ch_ssh", &state->ssh_tree);
ret = nc_server_config_new_ch_del_ch_client("ch_ssh", &state->ssh_tree);
assert_int_equal(ret, 0);

/* new poll session */
Expand Down Expand Up @@ -282,7 +282,7 @@ server_thread_tls(void *arg)
struct nc_pollsession *ps;

/* prepare data for deleting the call-home client */
ret = nc_server_config_new_del_ch_client("ch_tls", &state->tls_tree);
ret = nc_server_config_new_ch_del_ch_client("ch_tls", &state->tls_tree);
assert_int_equal(ret, 0);

/* new poll session */
Expand Down

0 comments on commit 94ec228

Please sign in to comment.