cluster: STABLE31 - rgmanager: Fix errors found during static analysis

Lon Hohberger lon at fedoraproject.org
Thu Jun 23 17:44:38 UTC 2011


Gitweb:        http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=72fca398e2a0cd5eb08f575b415cf815c73a84ad
Commit:        72fca398e2a0cd5eb08f575b415cf815c73a84ad
Parent:        e2937eb33f224f86904fead08499a6178868ca6a
Author:        Lon Hohberger <lhh at redhat.com>
AuthorDate:    Mon May 23 16:51:10 2011 -0400
Committer:     Lon Hohberger <lhh at redhat.com>
CommitterDate: Thu Jun 23 13:42:42 2011 -0400

rgmanager: Fix errors found during static analysis

Signed-off-by: Lon Hohberger <lhh at redhat.com>
---
 rgmanager/src/clulib/daemon_init.c   |    1 -
 rgmanager/src/clulib/lock.c          |    3 ---
 rgmanager/src/clulib/members.c       |    8 ++++++--
 rgmanager/src/clulib/message.c       |    2 +-
 rgmanager/src/clulib/msg_socket.c    |    4 +++-
 rgmanager/src/clulib/vft.c           |   30 ++++++++++++++----------------
 rgmanager/src/daemons/event_config.c |    4 +++-
 rgmanager/src/daemons/fo_domain.c    |   18 +++++++++++++-----
 rgmanager/src/daemons/main.c         |    4 ++++
 rgmanager/src/daemons/resrules.c     |    4 +++-
 rgmanager/src/daemons/restree.c      |    8 +++-----
 rgmanager/src/daemons/rg_event.c     |    6 ++++++
 rgmanager/src/daemons/rg_state.c     |   19 +++++++++++++------
 rgmanager/src/daemons/rg_thread.c    |    2 ++
 rgmanager/src/daemons/test.c         |    2 +-
 rgmanager/src/utils/clustat.c        |   10 ++++++----
 rgmanager/src/utils/clusvcadm.c      |    2 ++
 17 files changed, 80 insertions(+), 47 deletions(-)

diff --git a/rgmanager/src/clulib/daemon_init.c b/rgmanager/src/clulib/daemon_init.c
index e9adeed..7703d34 100644
--- a/rgmanager/src/clulib/daemon_init.c
+++ b/rgmanager/src/clulib/daemon_init.c
@@ -50,7 +50,6 @@ check_pid_valid(pid_t pid, char *prog)
 
 	snprintf(dirpath, sizeof (dirpath), "/proc/%d", pid);
 	if ((dir = opendir(dirpath)) == NULL) {
-		closedir(dir);
 		return 0;	/* Pid has gone away. */
 	}
 	closedir(dir);
diff --git a/rgmanager/src/clulib/lock.c b/rgmanager/src/clulib/lock.c
index be542c1..078d525 100644
--- a/rgmanager/src/clulib/lock.c
+++ b/rgmanager/src/clulib/lock.c
@@ -50,9 +50,6 @@ clu_ls_lock(dlm_lshandle_t ls,
         int ret;
 
 	if (!ls || !lksb || !resource || !strlen(resource)) {
-		printf("%p %p %p %d\n", ls, lksb, resource,
-		       (int)strlen(resource));
-		printf("INVAL...\n");
 		errno = EINVAL;
 		return -1;
 	}
diff --git a/rgmanager/src/clulib/members.c b/rgmanager/src/clulib/members.c
index 5f98708..e6475e1 100644
--- a/rgmanager/src/clulib/members.c
+++ b/rgmanager/src/clulib/members.c
@@ -282,14 +282,18 @@ get_member_list(cman_handle_t h)
 	}
 
 	do {	
+retry:
 		++tries;
-		if (nodes)
+		if (nodes) {
 			free(nodes);
+			nodes = NULL;
+		}
 
 		c = cman_get_node_count(h);
 		if (c <= 0) {
 			if (errno == EINTR)
-				continue;
+				/* continue if ml == NULL -> crash */
+				goto retry;
 			if (ml)
 				free(ml);
 			ml = NULL;
diff --git a/rgmanager/src/clulib/message.c b/rgmanager/src/clulib/message.c
index 3eb7b2f..441d421 100644
--- a/rgmanager/src/clulib/message.c
+++ b/rgmanager/src/clulib/message.c
@@ -259,7 +259,7 @@ msg_new_ctx(void)
 	if (!p)
 		return NULL;
 
-	memset(p, 0, sizeof(p));
+	memset(p, 0, sizeof(*p));
 	p->type = MSG_NONE;
 
 	return p;
diff --git a/rgmanager/src/clulib/msg_socket.c b/rgmanager/src/clulib/msg_socket.c
index a9c36d6..456858a 100644
--- a/rgmanager/src/clulib/msg_socket.c
+++ b/rgmanager/src/clulib/msg_socket.c
@@ -364,8 +364,10 @@ sock_msg_listen(int me, const void *portp, msgctx_t **listen_ctx)
 		return -1;
 
 	sock = socket(PF_LOCAL, SOCK_STREAM, 0);
-	if (sock < 0)
+	if (sock < 0) {
+		msg_free_ctx(ctx);
 		return -1;
+	}
 
 	set_cloexec(sock);
 	unlink(RGMGR_SOCK);
diff --git a/rgmanager/src/clulib/vft.c b/rgmanager/src/clulib/vft.c
index 0113bad..08dad42 100644
--- a/rgmanager/src/clulib/vft.c
+++ b/rgmanager/src/clulib/vft.c
@@ -755,13 +755,6 @@ vf_try_commit(key_node_t *key_node)
 	free(cmp); /* no need for it any longer */
 		
 	vnp = vn_remove(&key_node->kn_jvlist, trans);
-	if (!vnp) {
-		/*
-		 * But, we know it was the first element on the list?!!
-		 */
-		fprintf(stderr,"VF: QUAAAAAAAAAAAAAACKKKK!");
-		raise(SIGSTOP);
-	}
 	
 #ifdef DEBUG
 	printf("VF: Commit Key %s #%d from member #%d\n",
@@ -1201,6 +1194,7 @@ vf_write(cluster_member_list_t *membership, uint32_t flags,
 	 */
 	if (msg_open(MSG_CLUSTER, 0, _port, &everyone, 0) < 0) {
 		printf("msg_open: fail: %s\n", strerror(errno));
+		free(join_view);
 		clu_unlock(&lockp);
 		pthread_mutex_unlock(&vf_mutex);
 		return -1;
@@ -1495,16 +1489,20 @@ vf_read(cluster_member_list_t *membership, const char *keyid, uint64_t *view,
 		}
 	}
 
-	*data = malloc(key_node->kn_datalen);
-	if (! *data) {
-		pthread_mutex_unlock(&key_list_mutex);
-		clu_unlock(&lockp);
-		pthread_mutex_unlock(&vf_mutex);
-		printf("Couldn't malloc %s\n", keyid);
-		return VFR_ERROR;
-	}
+	if (key_node->kn_datalen && key_node->kn_data) {
+		*data = malloc(key_node->kn_datalen);
+		if (! *data) {
+			pthread_mutex_unlock(&key_list_mutex);
+			clu_unlock(&lockp);
+			pthread_mutex_unlock(&vf_mutex);
+			printf("Couldn't malloc %s\n", keyid);
+			return VFR_ERROR;
+		}
 
-	memcpy(*data, key_node->kn_data, key_node->kn_datalen);
+		memcpy(*data, key_node->kn_data, key_node->kn_datalen);
+	} else {
+		*data = NULL;
+	}
 	*datalen = key_node->kn_datalen;
 	*view = key_node->kn_viewno;
 
diff --git a/rgmanager/src/daemons/event_config.c b/rgmanager/src/daemons/event_config.c
index 4ab5bfc..9c4b8b3 100644
--- a/rgmanager/src/daemons/event_config.c
+++ b/rgmanager/src/daemons/event_config.c
@@ -291,8 +291,10 @@ get_event(int ccsfd, char *base, int idx, int *_done)
 	}
 
 	ev = malloc(sizeof(*ev));
-	if (!ev)
+	if (!ev) {
+		free(ret);
 		return NULL;
+	}
 	memset(ev, 0, sizeof(*ev));
 	ev->ev_name = ret;
 
diff --git a/rgmanager/src/daemons/fo_domain.c b/rgmanager/src/daemons/fo_domain.c
index 638dde9..884f3ae 100644
--- a/rgmanager/src/daemons/fo_domain.c
+++ b/rgmanager/src/daemons/fo_domain.c
@@ -46,7 +46,7 @@ fod_get_node(int __attribute__((unused)) ccsfd, char *base, int idx, fod_t *doma
 {
 	fod_node_t *fodn;
 	char xpath[256];
-	char *ret;
+	char *ret = NULL;
 
 	snprintf(xpath, sizeof(xpath), "%s/failoverdomainnode[%d]/@name",
 		 base, idx);
@@ -64,8 +64,10 @@ fod_get_node(int __attribute__((unused)) ccsfd, char *base, int idx, fod_t *doma
 	} while (!list_done(&domain->fd_nodes, fodn));
 
 	fodn = malloc(sizeof(*fodn));
-	if (!fodn)
+	if (!fodn) {
+		free(ret);
 		return NULL;
+	}
 	memset(fodn, 0, sizeof(*fodn));
 
 	/* Already malloc'd; simply store */
@@ -82,6 +84,7 @@ fod_get_node(int __attribute__((unused)) ccsfd, char *base, int idx, fod_t *doma
  	} else {
  		/* 64-bit-ism on rhel4? */
  		fodn->fdn_nodeid = atoi(ret);
+		free(ret);
  	}
  
  	/* Don't even bother getting priority if we're not ordered (it's set
@@ -344,7 +347,7 @@ int
 node_domain_set(fod_t **domains, char *name, int **ret, int *retlen, int *flags)
 {
 	int x, i, j;
-	int *tmpset;
+	int *tmpset = NULL;
 	int ts_count;
 	fod_node_t *fodn;
 	fod_t *domain;
@@ -368,8 +371,10 @@ node_domain_set(fod_t **domains, char *name, int **ret, int *retlen, int *flags)
 	if (!(*ret))
 		return -1;
 	tmpset = malloc(sizeof(int) * x);
-	if (!(*tmpset))
+	if (!tmpset) {
+		free(*ret);
 		return -1;
+	}
 
 	*flags = domain->fd_flags;
 
@@ -404,8 +409,10 @@ node_domain_set(fod_t **domains, char *name, int **ret, int *retlen, int *flags)
 		}
 	}
 
-	if (!ts_count)
+	if (!ts_count) {
+		free(tmpset);
 		return 0;
+	}
 
 	/* Shuffle stuff at this prio level */
 	if (ts_count > 1)
@@ -413,6 +420,7 @@ node_domain_set(fod_t **domains, char *name, int **ret, int *retlen, int *flags)
 	for (j = 0; j < ts_count; j++)
 		s_add(*ret, retlen, tmpset[j]);
 
+	free(tmpset);
 	return 0;
 }
 
diff --git a/rgmanager/src/daemons/main.c b/rgmanager/src/daemons/main.c
index a3ab7c4..7ee8cf9 100644
--- a/rgmanager/src/daemons/main.c
+++ b/rgmanager/src/daemons/main.c
@@ -654,7 +654,11 @@ static void
 dump_internal_state(const char *loc)
 {
 	FILE *fp;
+	if (!loc)
+		return;
 	fp=fopen(loc, "w+");
+	if (!fp)
+		return;
  	dump_config_version(fp);
  	dump_threads(fp);
  	dump_vf_states(fp);
diff --git a/rgmanager/src/daemons/resrules.c b/rgmanager/src/daemons/resrules.c
index 41bb6e8..fac48d9 100644
--- a/rgmanager/src/daemons/resrules.c
+++ b/rgmanager/src/daemons/resrules.c
@@ -1048,8 +1048,10 @@ load_resource_rulefile(char *filename, resource_rule_t **rules)
 		}
 
 		rr = malloc(sizeof(*rr));
-		if (!rr)
+		if (!rr) {
+			free(type);
 			break;
+		}
 		memset(rr,0,sizeof(*rr));
 
 		rr->rr_flags = RF_INIT | RF_DESTROY;
diff --git a/rgmanager/src/daemons/restree.c b/rgmanager/src/daemons/restree.c
index 3892ab9..739bbc8 100644
--- a/rgmanager/src/daemons/restree.c
+++ b/rgmanager/src/daemons/restree.c
@@ -497,10 +497,9 @@ assign_restart_policy(resource_t *curres, resource_node_t *parent,
 	time_t restart_expire_time = 0;
 	char tok[1024];
 
-	node->rn_restart_counter = NULL;
-
 	if (!curres || !node)
 		return;
+	node->rn_restart_counter = NULL;
 	if (parent &&
 	    !(node->rn_flags & RF_INDEPENDENT))
 		return;
@@ -1199,9 +1198,8 @@ mark_nodes(resource_node_t *node, int state, int setflags, int clearflags)
 	resource_node_t *child;
 
 	list_for(&node->rn_child, child, x) {
-		if (child)
-			mark_nodes(child, state, setflags,
-				   clearflags);
+		mark_nodes(child, state, setflags,
+			   clearflags);
 	}
 
 	if (state >= RES_STOPPED)
diff --git a/rgmanager/src/daemons/rg_event.c b/rgmanager/src/daemons/rg_event.c
index 7048bc6..6106e70 100644
--- a/rgmanager/src/daemons/rg_event.c
+++ b/rgmanager/src/daemons/rg_event.c
@@ -262,6 +262,7 @@ find_master(void)
 	} else {
 		masterinfo = (event_master_t *)data;
 	}
+
 	free_member_list(m);
 
 	if (masterinfo && (sz >= sizeof(*masterinfo))) {
@@ -275,7 +276,12 @@ find_master(void)
 			mi = masterinfo;
 			pthread_mutex_unlock(&mi_mutex);
 			master_id = masterinfo->m_nodeid;
+		} else {
+			free(data);
 		}
+	} else {
+		free(data);
+		masterinfo = NULL;
 	}
 
 	return master_id;
diff --git a/rgmanager/src/daemons/rg_state.c b/rgmanager/src/daemons/rg_state.c
index babac7a..1aff7a8 100644
--- a/rgmanager/src/daemons/rg_state.c
+++ b/rgmanager/src/daemons/rg_state.c
@@ -265,9 +265,12 @@ get_rg_state(const char *name, rg_state_t *svcblk)
 	uint32_t datalen = 0;
 #endif
 
-	/* ... */
-	if (name)
-		strncpy(svcblk->rs_name, name, sizeof(svcblk->rs_name));
+	if (!name) {
+		errno = EINVAL;
+		return -1;
+	}
+
+	strncpy(svcblk->rs_name, name, sizeof(svcblk->rs_name));
 
 	snprintf(res, sizeof(res),"rg=\"%s\"", svcblk->rs_name);
 
@@ -360,9 +363,11 @@ get_rg_state_local(const char *name, rg_state_t *svcblk)
 	uint32_t datalen;
 #endif
 
-	/* ... */
-	if (name)
-		strncpy(svcblk->rs_name, name, sizeof(svcblk->rs_name));
+	if (!name) {
+		errno = EINVAL;
+		return -1;
+	}
+	strncpy(svcblk->rs_name, name, sizeof(svcblk->rs_name));
 
 	snprintf(res, sizeof(res),"rg=\"%s\"", svcblk->rs_name);
 
@@ -422,6 +427,7 @@ svc_advise_stop(rg_state_t *svcStatus, const char *svcName, int req)
 	
 	if (svcStatus->rs_flags & RG_FLAG_FROZEN) {
 		logt_print(LOG_DEBUG, "Service %s frozen.\n", svcName);
+		free_member_list(membership);
 		return 5;
 	}
 
@@ -553,6 +559,7 @@ svc_advise_start(rg_state_t *svcStatus, const char *svcName, int req)
 	
 	if (svcStatus->rs_flags & RG_FLAG_FROZEN) {
 		logt_print(LOG_DEBUG, "Service %s frozen.\n", svcName);
+		free_member_list(membership);
 		return 5;
 	}
 
diff --git a/rgmanager/src/daemons/rg_thread.c b/rgmanager/src/daemons/rg_thread.c
index 72b5f96..75af6f1 100644
--- a/rgmanager/src/daemons/rg_thread.c
+++ b/rgmanager/src/daemons/rg_thread.c
@@ -523,10 +523,12 @@ resgroup_thread_main(void *arg)
 	/* reslist_mutex and my_queue_mutex held */
 	myself = find_resthread_byname(myname);
 
+	/* Not reached...
 	if (!myself) {
 		dbg_printf("I don't exist...\n");
 		raise(SIGSEGV);
 	}
+	 */
 
 	mystatus = pthread_mutex_destroy(&my_queue_mutex);
 	if (mystatus != 0)
diff --git a/rgmanager/src/daemons/test.c b/rgmanager/src/daemons/test.c
index 1982995..b0e326f 100644
--- a/rgmanager/src/daemons/test.c
+++ b/rgmanager/src/daemons/test.c
@@ -213,7 +213,7 @@ test_func(int argc, char **argv)
 		}
 		printf("Stop of %s complete\n", argv[3]);
 		goto out;
-	} else if (!strcmp(argv[1], "migrate")) {
+	} else if (!strcmp(argv[1], "migrate") && rn != NULL) {
 		printf("Migrating %s to %s...\n", argv[3], argv[4]);
 
 	#if 0
diff --git a/rgmanager/src/utils/clustat.c b/rgmanager/src/utils/clustat.c
index d7e8dd8..f736b51 100644
--- a/rgmanager/src/utils/clustat.c
+++ b/rgmanager/src/utils/clustat.c
@@ -276,7 +276,7 @@ rg_state_list(int local_node_id, int fast)
 		free(rsl);
 		return NULL;
 	}
-
+	
 	qsort(rsl->rgl_states, rsl->rgl_count, sizeof(rg_state_t),
 	      rg_name_sort);
 
@@ -352,10 +352,12 @@ static cluster_member_list_t *ccs_member_list(void)
 	}
 
 	ccs_disconnect(desc);
-	ret->cml_members = nodes;
 
-	qsort(ret->cml_members, ret->cml_count, sizeof(cman_node_t),
-	      member_id_sort);
+	ret->cml_members = nodes;
+	if (nodes) {
+		qsort(ret->cml_members, ret->cml_count,
+		      sizeof(cman_node_t), member_id_sort);
+	}
 
 	return ret;
 }
diff --git a/rgmanager/src/utils/clusvcadm.c b/rgmanager/src/utils/clusvcadm.c
index 8d0b502..3de9237 100644
--- a/rgmanager/src/utils/clusvcadm.c
+++ b/rgmanager/src/utils/clusvcadm.c
@@ -49,6 +49,8 @@ do_lock_req(int req)
 	int me;
 	generic_msg_hdr hdr;
 
+	ctx.type = -1;
+
 	ch = cman_init(NULL);
 	if (!ch) {
 		printf("Could not connect to cluster service\n");


More information about the cluster-commits mailing list