From 6eee473eba94697953e8b3e1b04fe5ef1de5f474 Mon Sep 17 00:00:00 2001 From: Amar Tumballi Date: Tue, 5 Jun 2012 14:15:54 +0530 Subject: core: coverity fixes (mostly resource leak fixes) currently working on obvious resource leak reports in coverity Change-Id: I261f4c578987b16da399ab5a504ad0fda0b176b1 Signed-off-by: Amar Tumballi BUG: 789278 Reviewed-on: http://review.gluster.com/3265 Tested-by: Gluster Build System Reviewed-by: Anand Avati --- cli/src/cli-cmd-parser.c | 17 ++++++-- cli/src/cli-rpc-ops.c | 4 ++ glusterfsd/src/glusterfsd-mgmt.c | 3 ++ glusterfsd/src/glusterfsd.c | 60 ++++++++++++++++++++------ libglusterfs/src/dict.c | 2 + libglusterfs/src/run.c | 8 +++- libglusterfs/src/statedump.c | 3 ++ libglusterfs/src/xlator.c | 3 ++ rpc/rpc-lib/src/rpc-transport.c | 20 +++++++-- rpc/rpc-lib/src/rpc-transport.h | 1 + rpc/rpc-lib/src/rpcsvc.c | 16 ++++--- rpc/rpc-transport/socket/src/socket.c | 2 +- xlators/cluster/dht/src/dht-helper.c | 2 +- xlators/cluster/dht/src/dht.c | 8 ++-- xlators/cluster/dht/src/switch.c | 37 +++++++++++----- xlators/cluster/stripe/src/stripe-helpers.c | 11 ++++- xlators/features/index/src/index.c | 2 +- xlators/mount/fuse/src/fuse-bridge.c | 2 +- xlators/protocol/server/src/authenticate.c | 1 + xlators/protocol/server/src/server-handshake.c | 5 ++- xlators/protocol/server/src/server-helpers.c | 4 ++ 21 files changed, 162 insertions(+), 49 deletions(-) diff --git a/cli/src/cli-cmd-parser.c b/cli/src/cli-cmd-parser.c index 02eb2c369..f447ccae3 100644 --- a/cli/src/cli-cmd-parser.c +++ b/cli/src/cli-cmd-parser.c @@ -367,7 +367,7 @@ cli_cmd_volume_create_parse (const char **words, int wordcount, dict_t **options ret = dict_set_dynstr (dict, "transport", trans_type); if (ret) goto out; - + trans_type = NULL; ret = dict_set_dynstr (dict, "bricks", bricks); if (ret) @@ -385,6 +385,10 @@ out: if (dict) dict_destroy (dict); } + + if (trans_type) + GF_FREE (trans_type); + return ret; } @@ -1576,8 +1580,12 @@ cli_cmd_gsync_set_parse (const char **words, int wordcount, dict_t **options) ret = dict_set_dynstr (dict, "op_value", append_str); } - if (!subop || dict_set_dynstr (dict, "subop", subop) != 0) - ret = -1; + ret = -1; + if (subop) { + ret = dict_set_dynstr (dict, "subop", subop); + if (!ret) + subop = NULL; + } } out: @@ -1589,6 +1597,9 @@ out: } else *options = dict; + if (subop) + GF_FREE (subop); + return ret; } diff --git a/cli/src/cli-rpc-ops.c b/cli/src/cli-rpc-ops.c index 7cc85e483..091c74f54 100644 --- a/cli/src/cli-rpc-ops.c +++ b/cli/src/cli-rpc-ops.c @@ -2780,6 +2780,7 @@ done: if (local) { local->dict = dict_ref (req_dict); frame->local = local; + local = NULL; } ret = dict_allocate_and_serialize (req_dict, @@ -2798,6 +2799,9 @@ done: (xdrproc_t) xdr_gf_cli_req); out: + if (local) + cli_local_wipe (local); + gf_log ("cli", GF_LOG_DEBUG, "Returning %d", ret); return ret; diff --git a/glusterfsd/src/glusterfsd-mgmt.c b/glusterfsd/src/glusterfsd-mgmt.c index d8582aa70..073ed54a0 100644 --- a/glusterfsd/src/glusterfsd-mgmt.c +++ b/glusterfsd/src/glusterfsd-mgmt.c @@ -1521,6 +1521,9 @@ glusterfs_volfile_reconfigure (FILE *newvolfile_fp) ret = 0; out: + if (oldvolfile_fp) + fclose (oldvolfile_fp); + return ret; } diff --git a/glusterfsd/src/glusterfsd.c b/glusterfsd/src/glusterfsd.c index ebd12bf0b..c6285b2ef 100644 --- a/glusterfsd/src/glusterfsd.c +++ b/glusterfsd/src/glusterfsd.c @@ -1025,6 +1025,7 @@ glusterfs_ctx_defaults_init (glusterfs_ctx_t *ctx) cmd_args_t *cmd_args = NULL; struct rlimit lim = {0, }; call_pool_t *pool = NULL; + int ret = -1; xlator_mem_acct_init (THIS, gfd_mt_end); @@ -1032,7 +1033,7 @@ glusterfs_ctx_defaults_init (glusterfs_ctx_t *ctx) if (!ctx->process_uuid) { gf_log ("", GF_LOG_CRITICAL, "ERROR: glusterfs uuid generation failed"); - return -1; + goto out; } ctx->page_size = 128 * GF_UNIT_KB; @@ -1041,14 +1042,14 @@ glusterfs_ctx_defaults_init (glusterfs_ctx_t *ctx) if (!ctx->iobuf_pool) { gf_log ("", GF_LOG_CRITICAL, "ERROR: glusterfs iobuf pool creation failed"); - return -1; + goto out; } ctx->event_pool = event_pool_new (DEFAULT_EVENT_POOL_SIZE); if (!ctx->event_pool) { gf_log ("", GF_LOG_CRITICAL, "ERROR: glusterfs event pool creation failed"); - return -1; + goto out; } pool = GF_CALLOC (1, sizeof (call_pool_t), @@ -1056,7 +1057,7 @@ glusterfs_ctx_defaults_init (glusterfs_ctx_t *ctx) if (!pool) { gf_log ("", GF_LOG_CRITICAL, "ERROR: glusterfs call pool creation failed"); - return -1; + goto out; } /* frame_mem_pool size 112 * 4k */ @@ -1064,21 +1065,21 @@ glusterfs_ctx_defaults_init (glusterfs_ctx_t *ctx) if (!pool->frame_mem_pool) { gf_log ("", GF_LOG_CRITICAL, "ERROR: glusterfs frame pool creation failed"); - return -1; + goto out; } /* stack_mem_pool size 256 * 1024 */ pool->stack_mem_pool = mem_pool_new (call_stack_t, 1024); if (!pool->stack_mem_pool) { gf_log ("", GF_LOG_CRITICAL, "ERROR: glusterfs stack pool creation failed"); - return -1; + goto out; } ctx->stub_mem_pool = mem_pool_new (call_stub_t, 1024); if (!ctx->stub_mem_pool) { gf_log ("", GF_LOG_CRITICAL, "ERROR: glusterfs stub pool creation failed"); - return -1; + goto out; } ctx->dict_pool = mem_pool_new (dict_t, GF_MEMPOOL_COUNT_OF_DICT_T); @@ -1123,7 +1124,35 @@ glusterfs_ctx_defaults_init (glusterfs_ctx_t *ctx) lim.rlim_max = RLIM_INFINITY; setrlimit (RLIMIT_CORE, &lim); - return 0; + ret = 0; +out: + + if (ret && pool) { + + if (pool->frame_mem_pool) + mem_pool_destroy (pool->frame_mem_pool); + + if (pool->stack_mem_pool) + mem_pool_destroy (pool->stack_mem_pool); + + GF_FREE (pool); + } + + if (ret && ctx) { + if (ctx->stub_mem_pool) + mem_pool_destroy (ctx->stub_mem_pool); + + if (ctx->dict_pool) + mem_pool_destroy (ctx->dict_pool); + + if (ctx->dict_data_pool) + mem_pool_destroy (ctx->dict_data_pool); + + if (ctx->dict_pair_pool) + mem_pool_destroy (ctx->dict_pair_pool); + } + + return ret; } static int @@ -1273,7 +1302,7 @@ int glusterfs_pidfile_setup (glusterfs_ctx_t *ctx) { cmd_args_t *cmd_args = NULL; - int ret = 0; + int ret = -1; FILE *pidfp = NULL; cmd_args = &ctx->cmd_args; @@ -1286,7 +1315,7 @@ glusterfs_pidfile_setup (glusterfs_ctx_t *ctx) gf_log ("glusterfsd", GF_LOG_ERROR, "pidfile %s error (%s)", cmd_args->pid_file, strerror (errno)); - return -1; + goto out; } ret = lockf (fileno (pidfp), F_TLOCK, 0); @@ -1294,7 +1323,7 @@ glusterfs_pidfile_setup (glusterfs_ctx_t *ctx) gf_log ("glusterfsd", GF_LOG_ERROR, "pidfile %s lock error (%s)", cmd_args->pid_file, strerror (errno)); - return ret; + goto out; } gf_log ("glusterfsd", GF_LOG_TRACE, @@ -1306,12 +1335,17 @@ glusterfs_pidfile_setup (glusterfs_ctx_t *ctx) gf_log ("glusterfsd", GF_LOG_ERROR, "pidfile %s unlock error (%s)", cmd_args->pid_file, strerror (errno)); - return ret; + goto out; } ctx->pidfp = pidfp; - return 0; + ret = 0; +out: + if (ret && pidfp) + fclose (pidfp); + + return ret; } diff --git a/libglusterfs/src/dict.c b/libglusterfs/src/dict.c index 804841470..5ae61fd3d 100644 --- a/libglusterfs/src/dict.c +++ b/libglusterfs/src/dict.c @@ -265,6 +265,8 @@ _dict_set (dict_t *this, if (this->free_pair_in_use) { pair = mem_get0 (THIS->ctx->dict_pair_pool); if (!pair) { + if (key_free) + GF_FREE (key); return -1; } } diff --git a/libglusterfs/src/run.c b/libglusterfs/src/run.c index 34d75df69..50c1e30bf 100644 --- a/libglusterfs/src/run.c +++ b/libglusterfs/src/run.c @@ -67,7 +67,10 @@ runner_chio (runner_t *runner, int fd) { GF_ASSERT (fd > 0 && fd < 3); - return runner->chio[fd]; + if ((fd > 0) && (fd < 3)) + return runner->chio[fd]; + + return NULL; } static void @@ -194,7 +197,8 @@ runner_redir (runner_t *runner, int fd, int tgt_fd) { GF_ASSERT (fd > 0 && fd < 3); - runner->chfd[fd] = (tgt_fd >= 0) ? tgt_fd : -2; + if ((fd > 0) && (fd < 3)) + runner->chfd[fd] = (tgt_fd >= 0) ? tgt_fd : -2; } int diff --git a/libglusterfs/src/statedump.c b/libglusterfs/src/statedump.c index 899d8ef2b..694146874 100644 --- a/libglusterfs/src/statedump.c +++ b/libglusterfs/src/statedump.c @@ -608,6 +608,9 @@ gf_proc_dump_options_init () } + if (fp) + fclose (fp); + return 0; } diff --git a/libglusterfs/src/xlator.c b/libglusterfs/src/xlator.c index 470087df9..7c081fa55 100644 --- a/libglusterfs/src/xlator.c +++ b/libglusterfs/src/xlator.c @@ -168,6 +168,9 @@ xlator_volopt_dynload (char *xlator_type, void **dl_handle, ret = 0; out: + if (name) + GF_FREE (name); + gf_log ("xlator", GF_LOG_DEBUG, "Returning %d", ret); return ret; diff --git a/rpc/rpc-lib/src/rpc-transport.c b/rpc/rpc-lib/src/rpc-transport.c index 8da898b61..d0e7834e8 100644 --- a/rpc/rpc-lib/src/rpc-transport.c +++ b/rpc/rpc-lib/src/rpc-transport.c @@ -257,6 +257,8 @@ rpc_transport_load (glusterfs_ctx_t *ctx, dict_t *options, char *trans_name) goto fail; } + trans->dl_handle = handle; + trans->ops = dlsym (handle, "tops"); if (trans->ops == NULL) { gf_log ("rpc-transport", GF_LOG_ERROR, @@ -319,9 +321,11 @@ rpc_transport_load (glusterfs_ctx_t *ctx, dict_t *options, char *trans_name) return_trans = trans; - if (name) { + if (name) GF_FREE (name); - } + + if (vol_opt) + GF_FREE (vol_opt); return return_trans; @@ -331,12 +335,17 @@ fail: GF_FREE (trans->name); } + if (trans->dl_handle) + dlclose (trans->dl_handle); + GF_FREE (trans); } - if (name) { + if (name) GF_FREE (name); - } + + if (vol_opt) + GF_FREE (vol_opt); return NULL; } @@ -426,6 +435,9 @@ rpc_transport_destroy (rpc_transport_t *this) if (this->name) GF_FREE (this->name); + if (this->dl_handle) + dlclose (this->dl_handle); + GF_FREE (this); fail: return ret; diff --git a/rpc/rpc-lib/src/rpc-transport.h b/rpc/rpc-lib/src/rpc-transport.h index d9ab30dd8..91d802220 100644 --- a/rpc/rpc-lib/src/rpc-transport.h +++ b/rpc/rpc-lib/src/rpc-transport.h @@ -205,6 +205,7 @@ struct rpc_transport { struct list_head list; int bind_insecure; + void *dl_handle; /* handle of dlopen() */ }; struct rpc_transport_ops { diff --git a/rpc/rpc-lib/src/rpcsvc.c b/rpc/rpc-lib/src/rpcsvc.c index 98cc88d63..ee9e1c725 100644 --- a/rpc/rpc-lib/src/rpcsvc.c +++ b/rpc/rpc-lib/src/rpcsvc.c @@ -1502,6 +1502,7 @@ rpcsvc_create_listeners (rpcsvc_t *svc, dict_t *options, char *name) } GF_FREE (transport_name); + transport_name = NULL; count++; } @@ -1513,17 +1514,17 @@ rpcsvc_create_listeners (rpcsvc_t *svc, dict_t *options, char *name) transport_type = NULL; out: - if (str != NULL) { + if (str) GF_FREE (str); - } - if (transport_type != NULL) { + if (transport_type) GF_FREE (transport_type); - } - if (tmp != NULL) { + if (tmp) GF_FREE (tmp); - } + + if (transport_name) + GF_FREE (transport_name); return count; } @@ -2398,6 +2399,9 @@ rpcsvc_transport_privport_check (rpcsvc_t *svc, char *volname, " allowed"); err: + if (srchstr) + GF_FREE (srchstr); + return ret; } diff --git a/rpc/rpc-transport/socket/src/socket.c b/rpc/rpc-transport/socket/src/socket.c index d193c93ce..cf30e7d7d 100644 --- a/rpc/rpc-transport/socket/src/socket.c +++ b/rpc/rpc-transport/socket/src/socket.c @@ -314,7 +314,7 @@ __socket_server_bind (rpc_transport_t *this) memcpy (&unix_addr, SA (&this->myinfo.sockaddr), this->myinfo.sockaddr_len); reuse_check_sock = socket (AF_UNIX, SOCK_STREAM, 0); - if (reuse_check_sock > 0) { + if (reuse_check_sock >= 0) { ret = connect (reuse_check_sock, SA (&unix_addr), this->myinfo.sockaddr_len); if ((ret == -1) && (ECONNREFUSED == errno)) { diff --git a/xlators/cluster/dht/src/dht-helper.c b/xlators/cluster/dht/src/dht-helper.c index 920a7aabc..611de19e4 100644 --- a/xlators/cluster/dht/src/dht-helper.c +++ b/xlators/cluster/dht/src/dht-helper.c @@ -80,7 +80,7 @@ dht_filter_loc_subvol_key (xlator_t *this, loc_t *loc, loc_t *new_loc, int ret = 0; /* not found */ /* Why do other tasks if first required 'char' itself is not there */ - if (!loc->name || !strchr (loc->name, '@')) + if (!new_loc || !loc || !loc->name || !strchr (loc->name, '@')) goto out; trav = this->children; diff --git a/xlators/cluster/dht/src/dht.c b/xlators/cluster/dht/src/dht.c index c25cdb4fd..c51285442 100644 --- a/xlators/cluster/dht/src/dht.c +++ b/xlators/cluster/dht/src/dht.c @@ -391,6 +391,8 @@ init (xlator_t *this) defrag->is_exiting = 0; + conf->defrag = defrag; + ret = dict_get_str (this->options, "node-uuid", &node_uuid); if (ret) { gf_log (this->name, GF_LOG_ERROR, "node-uuid not " @@ -407,9 +409,6 @@ init (xlator_t *this) defrag->cmd = cmd; defrag->stats = _gf_false; - - conf->defrag = defrag; - } conf->search_unhashed = GF_DHT_LOOKUP_UNHASHED_ON; @@ -493,6 +492,9 @@ err: if (conf->du_stats) GF_FREE (conf->du_stats); + if (conf->defrag) + GF_FREE (conf->defrag); + GF_FREE (conf); } diff --git a/xlators/cluster/dht/src/switch.c b/xlators/cluster/dht/src/switch.c index fe75914f2..ab261da87 100644 --- a/xlators/cluster/dht/src/switch.c +++ b/xlators/cluster/dht/src/switch.c @@ -67,29 +67,38 @@ get_switch_matching_subvol (const char *path, dht_conf_t *conf, struct switch_struct *cond = NULL; struct switch_struct *trav = NULL; char *pathname = NULL; - int idx = 0; + int idx = 0; + xlator_t *subvol = NULL; cond = conf->private; + subvol = hashed_subvol; if (!cond) - return hashed_subvol; + goto out; - trav = cond; pathname = gf_strdup (path); + if (!pathname) + goto out; + + trav = cond; while (trav) { if (fnmatch (trav->path_pattern, pathname, FNM_NOESCAPE) == 0) { for (idx = 0; idx < trav->num_child; idx++) { if (trav->array[idx].xl == hashed_subvol) - return hashed_subvol; + goto out; } idx = trav->node_index++; trav->node_index %= trav->num_child; - return trav->array[idx].xl; + subvol = trav->array[idx].xl; + goto out; } trav = trav->next; } - GF_FREE (pathname); - return hashed_subvol; +out: + if (pathname) + GF_FREE (pathname); + + return subvol; } @@ -663,8 +672,10 @@ set_switch_pattern (xlator_t *this, dht_conf_t *conf, dup_str = gf_strdup (switch_str); switch_opt = GF_CALLOC (1, sizeof (struct switch_struct), gf_switch_mt_switch_struct); - if (!switch_opt) + if (!switch_opt) { + GF_FREE (dup_str); goto err; + } pattern = strtok_r (dup_str, ":", &tmp_str1); childs = strtok_r (NULL, ":", &tmp_str1); @@ -674,6 +685,7 @@ set_switch_pattern (xlator_t *this, dht_conf_t *conf, "for all the unconfigured child nodes," " hence neglecting current option"); switch_str = strtok_r (NULL, ";", &tmp_str); + GF_FREE (switch_opt); GF_FREE (dup_str); continue; } @@ -746,6 +758,7 @@ set_switch_pattern (xlator_t *this, dht_conf_t *conf, /* First entry */ switch_buf = switch_opt; } + switch_opt = NULL; switch_str = strtok_r (NULL, ";", &tmp_str); } @@ -802,15 +815,19 @@ set_switch_pattern (xlator_t *this, dht_conf_t *conf, /* First entry */ switch_buf = switch_opt; } + switch_opt = NULL; } /* */ conf->private = switch_buf; return 0; err: + if (switch_buf_array) + GF_FREE (switch_buf_array); + if (switch_opt) + GF_FREE (switch_opt); + if (switch_buf) { - if (switch_buf_array) - GF_FREE (switch_buf_array); trav = switch_buf; while (trav) { if (trav->array) diff --git a/xlators/cluster/stripe/src/stripe-helpers.c b/xlators/cluster/stripe/src/stripe-helpers.c index a2ebc1201..1821832c2 100644 --- a/xlators/cluster/stripe/src/stripe-helpers.c +++ b/xlators/cluster/stripe/src/stripe-helpers.c @@ -471,12 +471,16 @@ set_stripe_block_size (xlator_t *this, stripe_private_t *priv, char *data) temp_stripeopt = NULL; else temp_stripeopt = priv->pattern; - priv->pattern = stripe_opt; + stripe_opt->next = temp_stripeopt; - stripe_str = strtok_r (NULL, ",", &tmp_str); + priv->pattern = stripe_opt; + stripe_opt = NULL; + GF_FREE (dup_str); dup_str = NULL; + + stripe_str = strtok_r (NULL, ",", &tmp_str); } ret = 0; @@ -485,6 +489,9 @@ out: if (dup_str) GF_FREE (dup_str); + if (stripe_opt) + GF_FREE (stripe_opt); + return ret; } diff --git a/xlators/features/index/src/index.c b/xlators/features/index/src/index.c index 1025c3fc3..508eb91a3 100644 --- a/xlators/features/index/src/index.c +++ b/xlators/features/index/src/index.c @@ -1070,13 +1070,13 @@ init (xlator_t *this) uuid_generate (priv->xattrop_vgfid); INIT_LIST_HEAD (&priv->callstubs); - this->private = priv; ret = pthread_create (&thread, &priv->w_attr, index_worker, this); if (ret) { gf_log (this->name, GF_LOG_WARNING, "Failed to create " "worker thread, aborting"); goto out; } + this->private = priv; ret = 0; out: if (!this->private && priv) diff --git a/xlators/mount/fuse/src/fuse-bridge.c b/xlators/mount/fuse/src/fuse-bridge.c index be394773b..6d39b1f5a 100644 --- a/xlators/mount/fuse/src/fuse-bridge.c +++ b/xlators/mount/fuse/src/fuse-bridge.c @@ -106,7 +106,7 @@ send_fuse_iov (xlator_t *this, fuse_in_header_t *finh, struct iovec *iov_out, if (!this || !finh || !iov_out) { gf_log ("send_fuse_iov", GF_LOG_ERROR,"Invalid arguments"); - return -1; + return EINVAL; } priv = this->private; diff --git a/xlators/protocol/server/src/authenticate.c b/xlators/protocol/server/src/authenticate.c index ffadf2e4d..d1cdebdee 100644 --- a/xlators/protocol/server/src/authenticate.c +++ b/xlators/protocol/server/src/authenticate.c @@ -96,6 +96,7 @@ init (dict_t *this, char *key, data_t *value, void *data) if (!auth_handle->vol_opt) { dict_set (this, key, data_from_dynptr (NULL, 0)); *error = -1; + GF_FREE (auth_handle); dlclose (handle); return; } diff --git a/xlators/protocol/server/src/server-handshake.c b/xlators/protocol/server/src/server-handshake.c index 7761f78c4..abccc3898 100644 --- a/xlators/protocol/server/src/server-handshake.c +++ b/xlators/protocol/server/src/server-handshake.c @@ -311,8 +311,6 @@ server_getspec (rpcsvc_request_t *req) goto fail; } ret = read (spec_fd, rsp.spec, file_len); - - close (spec_fd); } /* convert to XDR */ @@ -323,6 +321,9 @@ fail: rsp.op_errno = gf_errno_to_error (op_errno); rsp.op_ret = ret; + if (spec_fd != -1) + close (spec_fd); + server_submit_reply (NULL, req, &rsp, NULL, 0, NULL, (xdrproc_t)xdr_gf_getspec_rsp); diff --git a/xlators/protocol/server/src/server-helpers.c b/xlators/protocol/server/src/server-helpers.c index 8827a48b0..284b44145 100644 --- a/xlators/protocol/server/src/server-helpers.c +++ b/xlators/protocol/server/src/server-helpers.c @@ -1289,10 +1289,14 @@ serialize_rsp_direntp (gf_dirent_t *entries, gfs3_readdirp_rsp *rsp) rsp->reply = trav; prev = trav; + trav = NULL; } ret = 0; out: + if (trav) + GF_FREE (trav); + return ret; } -- cgit