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

rgmanager: Fix errors found during static analysis

Signed-off-by: Lon Hohberger lhh@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");