From: rgirada <rgirada@vmware.com>
Date: Fri, 24 Mar 2023 10:09:42 +0000 (+0000)
Subject: mgmtd: Fixing code coverity issues in mgmtd
X-Git-Tag: base_9.0~194^2~1
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=93d4e355d87ed6211031d22cd67f229246ec98ad;p=matthieu%2Ffrr.git

mgmtd: Fixing code coverity issues in mgmtd

Description:
	the following list of coverity issues seen in mgmtd code.
	1. CID 1536832:  Memory - corruptions  (OVERLAPPING_COPY)
	   /mgmtd/mgmt_history.c: 85 in mgmt_history_create_cmt_rec()
	2. CID 1536831:  Error handling issues  (NEGATIVE_RETURNS)
	   /mgmtd/mgmt_be_server.c: 123 in mgmt_be_server_start()
	3. CID 1536830:  Resource leaks  (RESOURCE_LEAK)
	   /mgmtd/mgmt_history.c: 146 in mgmt_history_read_cmt_record_index()
	4. CID 1536829:  Error handling issues  (NEGATIVE_RETURNS)
	   /mgmtd/mgmt_fe_server.c: 123 in mgmt_fe_server_start()
	5. CID 1536828:  Possible Control flow issues  (DEADCODE)
	   /mgmtd/mgmt_txn.c: 1859 in mgmt_txn_get_config()
	6. CID 1536827:  Null pointer dereferences  (NULL_RETURNS)
           /mgmtd/mgmt_ds.c: 526 in mgmt_ds_delete_data_nodes()

Signed-off-by: Rajesh Girada <rgirada@vmware.com>
---

diff --git a/mgmtd/mgmt_be_server.c b/mgmtd/mgmt_be_server.c
index 0fa7ddd6d6..aa77464524 100644
--- a/mgmtd/mgmt_be_server.c
+++ b/mgmtd/mgmt_be_server.c
@@ -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;
diff --git a/mgmtd/mgmt_ds.c b/mgmtd/mgmt_ds.c
index 10b3cecb92..b41b9d23d3 100644
--- a/mgmtd/mgmt_ds.c
+++ b/mgmtd/mgmt_ds.c
@@ -499,12 +499,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);
diff --git a/mgmtd/mgmt_fe_server.c b/mgmtd/mgmt_fe_server.c
index 6097c23aac..e8bbe139bb 100644
--- a/mgmtd/mgmt_fe_server.c
+++ b/mgmtd/mgmt_fe_server.c
@@ -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;
diff --git a/mgmtd/mgmt_history.c b/mgmtd/mgmt_history.c
index 6f9f2dd63f..533b68628e 100644
--- a/mgmtd/mgmt_history.c
+++ b/mgmtd/mgmt_history.c
@@ -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) {
diff --git a/mgmtd/mgmt_txn.c b/mgmtd/mgmt_txn.c
index 5fa8aabfd6..25943ff742 100644
--- a/mgmtd/mgmt_txn.c
+++ b/mgmtd/mgmt_txn.c
@@ -1787,27 +1787,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 +1835,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;
 }