Merge pull request #13122 from rgirada/mgmtd_codecov

mgmtd: Fixing coverity issues and style warnings in the code
This commit is contained in:
Jafar Al-Gharaibeh 2023-04-12 00:05:56 -05:00 committed by GitHub
commit b4ac6683ac
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 74 additions and 87 deletions

View File

@ -16,17 +16,17 @@
#include "sockopt.h"
#ifdef REDIRECT_DEBUG_TO_STDERR
#define MGMTD_BE_CLIENT_DBG(fmt, ...) \
#define MGMTD_BE_CLIENT_DBG(fmt, ...) \
fprintf(stderr, "%s: " fmt "\n", __func__, ##__VA_ARGS__)
#define MGMTD_BE_CLIENT_ERR(fmt, ...) \
#define MGMTD_BE_CLIENT_ERR(fmt, ...) \
fprintf(stderr, "%s: ERROR, " fmt "\n", __func__, ##__VA_ARGS__)
#else /* REDIRECT_DEBUG_TO_STDERR */
#define MGMTD_BE_CLIENT_DBG(fmt, ...) \
#define MGMTD_BE_CLIENT_DBG(fmt, ...) \
do { \
if (mgmt_debug_be_client) \
zlog_debug("%s: " fmt, __func__, ##__VA_ARGS__); \
if (mgmt_debug_be_client) \
zlog_debug("%s: " fmt, __func__, ##__VA_ARGS__); \
} while (0)
#define MGMTD_BE_CLIENT_ERR(fmt, ...) \
#define MGMTD_BE_CLIENT_ERR(fmt, ...) \
zlog_err("%s: ERROR: " fmt, __func__, ##__VA_ARGS__)
#endif /* REDIRECT_DEBUG_TO_STDERR */
@ -597,7 +597,8 @@ static int mgmt_be_txn_cfg_prepare(struct mgmt_be_txn_ctx *txn)
MGMTD_BE_CLIENT_DBG(
"Avg-nb-edit-duration %lu uSec, nb-prep-duration %lu (avg: %lu) uSec, batch size %u",
client_ctx->avg_edit_nb_cfg_tm, prep_nb_cfg_tm,
client_ctx->avg_prep_nb_cfg_tm, (uint32_t)num_processed);
client_ctx->avg_prep_nb_cfg_tm,
(uint32_t)num_processed);
if (error)
mgmt_be_txn_cfg_abort(txn);

View File

@ -109,8 +109,8 @@ mgmt_fe_find_session_by_session_id(struct mgmt_fe_client_ctx *client_ctx,
FOREACH_SESSION_IN_LIST (client_ctx, session) {
if (session->session_id == session_id) {
MGMTD_FE_CLIENT_DBG(
"Found session %p for session-id %llu.", session,
(unsigned long long)session_id);
"Found session %p for session-id %llu.",
session, (unsigned long long)session_id);
return session;
}
}
@ -274,7 +274,8 @@ mgmt_fe_send_lockds_req(struct mgmt_fe_client_ctx *client_ctx,
MGMTD_FE_CLIENT_DBG(
"Sending %sLOCK_REQ message for Ds:%d session %llu to MGMTD Frontend server",
lock ? "" : "UN", ds_id, (unsigned long long)session->client_id);
lock ? "" : "UN", ds_id,
(unsigned long long)session->client_id);
return mgmt_fe_client_send_msg(client_ctx, &fe_msg);
}
@ -310,12 +311,12 @@ mgmt_fe_send_setcfg_req(struct mgmt_fe_client_ctx *client_ctx,
return mgmt_fe_client_send_msg(client_ctx, &fe_msg);
}
static int
mgmt_fe_send_commitcfg_req(struct mgmt_fe_client_ctx *client_ctx,
struct mgmt_fe_client_session *session,
uint64_t req_id, Mgmtd__DatastoreId src_ds_id,
Mgmtd__DatastoreId dest_ds_id, bool validate_only,
bool abort)
static int mgmt_fe_send_commitcfg_req(struct mgmt_fe_client_ctx *client_ctx,
struct mgmt_fe_client_session *session,
uint64_t req_id,
Mgmtd__DatastoreId src_ds_id,
Mgmtd__DatastoreId dest_ds_id,
bool validate_only, bool abort)
{
(void)req_id;
Mgmtd__FeMessage fe_msg;
@ -450,15 +451,17 @@ mgmt_fe_client_handle_msg(struct mgmt_fe_client_ctx *client_ctx,
if (session && fe_msg->session_reply->success) {
MGMTD_FE_CLIENT_DBG(
"Session Create for client-id %llu successful.",
(unsigned long long)fe_msg
->session_reply->client_conn_id);
(unsigned long long)
fe_msg->session_reply
->client_conn_id);
session->session_id =
fe_msg->session_reply->session_id;
} else {
MGMTD_FE_CLIENT_ERR(
"Session Create for client-id %llu failed.",
(unsigned long long)fe_msg
->session_reply->client_conn_id);
(unsigned long long)
fe_msg->session_reply
->client_conn_id);
}
} else if (!fe_msg->session_reply->create) {
MGMTD_FE_CLIENT_DBG(

View File

@ -80,23 +80,23 @@ static const struct mgmt_be_xpath_map_reg xpath_static_map_reg[] = {
.be_clients =
(enum mgmt_be_client_id[]){
#if HAVE_STATICD
MGMTD_BE_CLIENT_ID_STATICD,
MGMTD_BE_CLIENT_ID_STATICD,
#endif
MGMTD_BE_CLIENT_ID_MAX}},
{.xpath_regexp = "/frr-interface:lib/*",
.be_clients =
(enum mgmt_be_client_id[]){
#if HAVE_STATICD
MGMTD_BE_CLIENT_ID_STATICD,
MGMTD_BE_CLIENT_ID_STATICD,
#endif
MGMTD_BE_CLIENT_ID_MAX}},
{.xpath_regexp =
"/frr-routing:routing/control-plane-protocols/control-plane-protocol[type='frr-staticd:staticd'][name='staticd'][vrf='default']/frr-staticd:staticd/*",
"/frr-routing:routing/control-plane-protocols/control-plane-protocol[type='frr-staticd:staticd'][name='staticd'][vrf='default']/frr-staticd:staticd/*",
.be_clients =
(enum mgmt_be_client_id[]){
#if HAVE_STATICD
MGMTD_BE_CLIENT_ID_STATICD,
MGMTD_BE_CLIENT_ID_STATICD,
#endif
MGMTD_BE_CLIENT_ID_MAX}},
};
@ -347,8 +347,8 @@ mgmt_be_adapter_cleanup_old_conn(struct mgmt_be_client_adapter *adapter)
struct mgmt_be_client_adapter *old;
FOREACH_ADAPTER_IN_LIST (old) {
if (old != adapter
&& !strncmp(adapter->name, old->name, sizeof(adapter->name))) {
if (old != adapter &&
!strncmp(adapter->name, old->name, sizeof(adapter->name))) {
/*
* We have a Zombie lingering around
*/

View File

@ -119,7 +119,7 @@ static void mgmt_be_server_start(const char *hostname)
return;
mgmt_be_server_start_failed:
if (sock)
if (sock > 0)
close(sock);
mgmt_be_listen_fd = -1;

View File

@ -174,6 +174,7 @@ static int mgmt_ds_load_cfg_from_file(const char *filepath,
void mgmt_ds_reset_candidate(void)
{
struct lyd_node *dnode = mm->candidate_ds->root.cfg_root->dnode;
if (dnode)
yang_dnode_free(dnode);
@ -499,12 +500,12 @@ int mgmt_ds_delete_data_nodes(struct mgmt_ds_ctx *ds_ctx, const char *xpath)
*/
return NB_ERR_NOT_FOUND;
/* destroy dependant */
if (nb_node->dep_cbs.get_dependant_xpath) {
if (nb_node && nb_node->dep_cbs.get_dependant_xpath) {
nb_node->dep_cbs.get_dependant_xpath(dnode, dep_xpath);
dep_dnode = yang_dnode_get(
ds_ctx->config_ds ? ds_ctx->root.cfg_root->dnode
: ds_ctx->root.dnode_root,
: ds_ctx->root.dnode_root,
dep_xpath);
if (dep_dnode)
lyd_free_tree(dep_dnode);

View File

@ -495,7 +495,8 @@ static int mgmt_fe_send_setcfg_reply(struct mgmt_fe_session_ctx *session,
if (implicit_commit) {
if (mm->perf_stats_en)
gettimeofday(&session->adapter->cmt_stats.last_end, NULL);
gettimeofday(&session->adapter->cmt_stats.last_end,
NULL);
mgmt_fe_session_compute_commit_timers(
&session->adapter->cmt_stats);
}
@ -715,8 +716,8 @@ mgmt_fe_adapter_cleanup_old_conn(struct mgmt_fe_client_adapter *adapter)
struct mgmt_fe_client_adapter *old;
FOREACH_ADAPTER_IN_LIST (old) {
if (old != adapter
&& !strncmp(adapter->name, old->name, sizeof(adapter->name))) {
if (old != adapter &&
!strncmp(adapter->name, old->name, sizeof(adapter->name))) {
/*
* We have a Zombie lingering around
*/
@ -1109,8 +1110,8 @@ mgmt_fe_session_handle_getdata_req_msg(struct mgmt_fe_session_ctx *session,
MGMTD_TXN_TYPE_SHOW);
if (session->txn_id == MGMTD_SESSION_ID_NONE) {
mgmt_fe_send_getdata_reply(
session, getdata_req->ds_id, getdata_req->req_id,
false, NULL,
session, getdata_req->ds_id,
getdata_req->req_id, false, NULL,
"Failed to create a Show transaction!");
goto mgmt_fe_sess_handle_getdata_req_failed;
}
@ -1780,8 +1781,8 @@ mgmt_fe_adapter_cmt_stats_write(struct vty *vty,
sizeof(buf)));
vty_out(vty, " Apply-Config Start: \t\t%s\n",
mgmt_realtime_to_string(
&adapter->cmt_stats.apply_cfg_start, buf,
sizeof(buf)));
&adapter->cmt_stats.apply_cfg_start,
buf, sizeof(buf)));
vty_out(vty, " Apply-Config End: \t\t%s\n",
mgmt_realtime_to_string(
&adapter->cmt_stats.apply_cfg_end, buf,
@ -1818,8 +1819,9 @@ mgmt_fe_adapter_setcfg_stats_write(struct vty *vty,
adapter->setcfg_stats.avg_tm);
vty_out(vty, " Last-Set-Cfg-Details:\n");
vty_out(vty, " Set-Cfg Start: \t\t\t%s\n",
mgmt_realtime_to_string(&adapter->setcfg_stats.last_start,
buf, sizeof(buf)));
mgmt_realtime_to_string(
&adapter->setcfg_stats.last_start, buf,
sizeof(buf)));
vty_out(vty, " Set-Cfg End: \t\t\t%s\n",
mgmt_realtime_to_string(&adapter->setcfg_stats.last_end,
buf, sizeof(buf)));
@ -1894,9 +1896,11 @@ void mgmt_fe_adapter_reset_perf_stats(struct vty *vty)
struct mgmt_fe_session_ctx *session;
FOREACH_ADAPTER_IN_LIST (adapter) {
memset(&adapter->setcfg_stats, 0, sizeof(adapter->setcfg_stats));
memset(&adapter->setcfg_stats, 0,
sizeof(adapter->setcfg_stats));
FOREACH_SESSION_IN_LIST (adapter, session) {
memset(&adapter->cmt_stats, 0, sizeof(adapter->cmt_stats));
memset(&adapter->cmt_stats, 0,
sizeof(adapter->cmt_stats));
}
}
}

View File

@ -119,7 +119,7 @@ static void mgmt_fe_server_start(const char *hostname)
return;
mgmt_fe_server_start_failed:
if (sock)
if (sock > 0)
close(sock);
mgmt_fe_listen_fd = -1;

View File

@ -33,7 +33,7 @@ DECLARE_DLIST(mgmt_cmt_infos, struct mgmt_cmt_info_t, cmts);
* The only instance of VTY session that has triggered an ongoing
* config rollback operation.
*/
static struct vty *rollback_vty = NULL;
static struct vty *rollback_vty;
static bool mgmt_history_record_exists(char *file_path)
{
@ -82,7 +82,7 @@ static struct mgmt_cmt_info_t *mgmt_history_create_cmt_rec(void)
mgmt_realtime_to_string(&cmt_recd_tv, new->time_str,
sizeof(new->time_str));
mgmt_history_hash(new->time_str, new->cmtid_str);
snprintf(new->cmt_json_file, sizeof(new->cmt_json_file),
snprintf(new->cmt_json_file, sizeof(new->cmt_json_file) - 1,
MGMTD_COMMIT_FILE_PATH, new->cmtid_str);
if (mgmt_cmt_infos_count(&mm->cmts) == MGMTD_MAX_COMMIT_LIST) {
@ -100,7 +100,8 @@ static struct mgmt_cmt_info_t *mgmt_history_create_cmt_rec(void)
return new;
}
static struct mgmt_cmt_info_t *mgmt_history_find_cmt_record(const char *cmtid_str)
static struct mgmt_cmt_info_t *
mgmt_history_find_cmt_record(const char *cmtid_str)
{
struct mgmt_cmt_info_t *cmt_info;
@ -129,7 +130,8 @@ static bool mgmt_history_read_cmt_record_index(void)
while ((fread(&cmt_info, sizeof(cmt_info), 1, fp)) > 0) {
if (cnt < MGMTD_MAX_COMMIT_LIST) {
if (!mgmt_history_record_exists(cmt_info.cmt_json_file)) {
if (!mgmt_history_record_exists(
cmt_info.cmt_json_file)) {
zlog_err(
"Commit record present in index_file, but commit file %s missing",
cmt_info.cmt_json_file);
@ -282,7 +284,8 @@ int mgmt_history_rollback_by_id(struct vty *vty, const char *cmtid_str)
FOREACH_CMT_REC (mm, cmt_info) {
if (strncmp(cmt_info->cmtid_str, cmtid_str,
MGMTD_MD5_HASH_STR_HEX_LEN) == 0) {
ret = mgmt_history_rollback_to_cmt(vty, cmt_info, false);
ret = mgmt_history_rollback_to_cmt(vty, cmt_info,
false);
return ret;
}
@ -321,7 +324,8 @@ int mgmt_history_rollback_n(struct vty *vty, int num_cmts)
FOREACH_CMT_REC (mm, cmt_info) {
if (cnt == num_cmts) {
ret = mgmt_history_rollback_to_cmt(vty, cmt_info, false);
ret = mgmt_history_rollback_to_cmt(vty, cmt_info,
false);
return ret;
}
@ -356,6 +360,7 @@ void show_mgmt_cmt_history(struct vty *vty)
void mgmt_history_new_record(struct mgmt_ds_ctx *ds_ctx)
{
struct mgmt_cmt_info_t *cmt_info = mgmt_history_create_cmt_rec();
mgmt_ds_dump_ds_to_file(cmt_info->cmt_json_file, ds_ctx);
mgmt_history_dump_cmt_record_index();
}

View File

@ -1,10 +1,10 @@
// SPDX-License-Identifier: GPL-2.0-or-later
/*
* Copyright (C) 2021 Vmware, Inc.
* Pushpasis Sarkar <spushpasis@vmware.com>
* Copyright (c) 2023, LabN Consulting, L.L.C.
*
*/
* Copyright (C) 2021 Vmware, Inc.
* Pushpasis Sarkar <spushpasis@vmware.com>
* Copyright (c) 2023, LabN Consulting, L.L.C.
*
*/
#ifndef _FRR_MGMTD_HISTORY_H_
#define _FRR_MGMTD_HISTORY_H_

View File

@ -1258,6 +1258,7 @@ static int mgmt_txn_prepare_config(struct mgmt_txn_ctx *txn)
char err_buf[1024] = {0};
nb_ctx.client = NB_CLIENT_MGMTD_SERVER;
nb_ctx.user = (void *)txn;
ret = nb_candidate_validate_yang(nb_config, false, err_buf,
sizeof(err_buf) - 1);
if (ret != NB_OK) {
@ -1787,27 +1788,10 @@ static int mgmt_txn_get_config(struct mgmt_txn_ctx *txn,
struct mgmt_txn_req *txn_req,
struct mgmt_ds_ctx *ds_ctx)
{
struct mgmt_txn_reqs_head *req_list = NULL;
struct mgmt_txn_reqs_head *pending_list = NULL;
int indx;
struct mgmt_get_data_req *get_data;
struct mgmt_get_data_reply *get_reply;
switch (txn_req->req_event) {
case MGMTD_TXN_PROC_GETCFG:
req_list = &txn->get_cfg_reqs;
break;
case MGMTD_TXN_PROC_GETDATA:
req_list = &txn->get_data_reqs;
break;
case MGMTD_TXN_PROC_SETCFG:
case MGMTD_TXN_PROC_COMMITCFG:
case MGMTD_TXN_COMMITCFG_TIMEOUT:
case MGMTD_TXN_CLEANUP:
assert(!"Wrong txn request type!");
break;
}
get_data = txn_req->req.get_data;
if (!get_data->reply) {
@ -1852,24 +1836,11 @@ static int mgmt_txn_get_config(struct mgmt_txn_ctx *txn,
mgmt_txn_get_config_failed:
if (pending_list) {
/*
* Move the transaction to corresponding pending list.
*/
if (req_list)
mgmt_txn_reqs_del(req_list, txn_req);
txn_req->pending_be_proc = true;
mgmt_txn_reqs_add_tail(pending_list, txn_req);
MGMTD_TXN_DBG(
"Moved Req: %p for Txn: %p from Req-List to Pending-List",
txn_req, txn_req->txn);
} else {
/*
* Delete the txn request. It will also remove it from request
* list.
*/
mgmt_txn_req_free(&txn_req);
}
/*
* Delete the txn request. It will also remove it from request
* list.
*/
mgmt_txn_req_free(&txn_req);
return 0;
}

View File

@ -391,6 +391,7 @@ static struct cmd_node debug_node = {
static int config_write_mgmt_debug_helper(struct vty *vty, bool config)
{
int n = mgmt_debug_be + mgmt_debug_fe + mgmt_debug_ds + mgmt_debug_txn;
if (!n)
return 0;
@ -442,6 +443,7 @@ DEFPY(debug_mgmt, debug_mgmt_cmd,
"Transaction debug\n")
{
bool set = !no;
if (all)
be = fe = ds = txn = set ? all : NULL;