From 115ecc8da8e79e02f23478a2b0721e783792d70e Mon Sep 17 00:00:00 2001 From: Lalatendu Mohanty Date: Wed, 30 Apr 2014 15:25:29 +0530 Subject: Core: Fix issues reported by Cppcheck Fixed in this patch: [glusterfs/extras/geo-rep/gsync-sync-gfid.c:105]: (error) Resource leak: fp [glusterfs/libglusterfs/src/xlator.c:651]: (error) Uninitialized variable: gfid [glusterfs/libglusterfs/src/xlator.c:652]: (error) Uninitialized variable: gfid [glusterfs/xlators/cluster/ha/src/ha.c:2699]: (error) Possible null pointer dereference: priv [glusterfs/xlators/features/changelog/src/changelog.c:1464]: (error) Possible null pointer dereference: priv [glusterfs/xlators/mgmt/glusterd/src/glusterd-mgmt-handler.c:865]: (error) Possible null pointer dereference: ctx [glusterfs/xlators/mgmt/glusterd/src/glusterd-mgmt-handler.c:194]: (error) Possible null pointer dereference: ctx [glusterfs/xlators/mgmt/glusterd/src/glusterd-syncop.c:1408]: (error) Possible null pointer dereference: this [glusterfs/xlators/mgmt/glusterd/src/glusterd-utils.c:7002]: (error) Possible null pointer dereference: path_tokens Fixed in 3.4 and 3.5 branch (http://review.gluster.org/#/c/7583/ , http://review.gluster.org/#/c/7605/ will be backported in a separate patch) [glusterfs/xlators/mount/fuse/src/fuse-bridge.c:4688]: (error) Uninitialized variable: finh [glusterfs/xlators/mount/fuse/src/fuse-bridge.c:3081]: (error) Possible null pointer dereference: state [glusterfs/xlators/cluster/dht/src/dht-rebalance.c:1719]: (error) Possible null pointer dereference: ctx [glusterfs/xlators/cluster/stripe/src/stripe.c:4940]: (error) Possible null pointer dereference: local [glusterfs/xlators/mgmt/glusterd/src/glusterd-replace-brick.c:915]: (error) Resource leak: file [glusterfs/xlators/mgmt/glusterd/src/glusterd-replace-brick.c:999]: (error) Resource leak: file [glusterfs/xlators/mgmt/glusterd/src/glusterd-sm.c:248]: (error) Possible null pointer dereference: new_ev_ctx [glusterfs/xlators/mgmt/glusterd/src/glusterd-utils.c:5297]: (error) Possible null pointer dereference: this [glusterfs/xlators/mgmt/glusterd/src/glusterd-utils.c:6273]: (error) Possible null pointer dereference: this [glusterfs/xlators/performance/quick-read/src/quick-read.c:586]: (error) Possible null pointer dereference: iobuf [glusterfs/xlators/nfs/server/src/nfs-common.c:89]: (error) Dangerous usage of 'volname' (strncpy doesn't always null-terminate it). False positives [glusterfs/geo-replication/src/gsyncd.c:99]: (error) Memory leak: str [glusterfs/geo-replication/src/gsyncd.c:395]: (error) Memory leak: argv [glusterfs/xlators/nfs/server/src/nlm4.c:1199]: (error) Possible null pointer dereference: fde [glusterfs/xlators/mgmt/glusterd/src/glusterd-geo-rep.c:1659]: (error) Possible null pointer dereference: command [glusterfs/xlators/mgmt/glusterd/src/glusterd-utils.c:7001]: (error) Possible null pointer dereference: path_tokens Insignificant/Don't care [glusterfs/contrib/uuid/gen_uuid.c:369]: (warning) %ld in format string (no. 2) requires 'long *' but the argument type is 'unsigned long *'. [glusterfs/contrib/uuid/gen_uuid.c:369]: (warning) %ld in format string (no. 3) requires 'long *' but the argument type is 'unsigned long *'. [glusterfs/extras/test/test-ffop.c:27]: (error) Buffer overrun possible for long command line arguments. [glusterfs/xlators/cluster/afr/src/afr-self-heal-common.c:138]: (error) Possible null pointer dereference: __ptr [glusterfs/xlators/cluster/afr/src/afr-self-heal-common.c:140]: (error) Possible null pointer dereference: __ptr [glusterfs/xlators/cluster/afr/src/afr-self-heal-common.c:331]: (error) Possible null pointer dereference: __ptr Change-Id: I7696ed1a2a9553b79f9714e10210a8d563a5abd8 BUG: 1091677 Signed-off-by: Lalatendu Mohanty Reviewed-on: http://review.gluster.org/7693 Tested-by: Gluster Build System Reviewed-by: Kaleb KEITHLEY Reviewed-by: Vijay Bellur --- extras/geo-rep/gsync-sync-gfid.c | 2 ++ libglusterfs/src/xlator.c | 2 +- xlators/cluster/ha/src/ha.c | 6 +++++- xlators/features/changelog/src/changelog.c | 23 ++++++++++++----------- xlators/mgmt/glusterd/src/glusterd-syncop.c | 3 ++- xlators/mgmt/glusterd/src/glusterd-utils.c | 7 ++++--- 6 files changed, 26 insertions(+), 17 deletions(-) diff --git a/extras/geo-rep/gsync-sync-gfid.c b/extras/geo-rep/gsync-sync-gfid.c index 953eb535132..e9b9e633402 100644 --- a/extras/geo-rep/gsync-sync-gfid.c +++ b/extras/geo-rep/gsync-sync-gfid.c @@ -102,6 +102,8 @@ main (int argc, char *argv[]) ret = 0; out: + if (fp) + fclose(fp); return ret; } diff --git a/libglusterfs/src/xlator.c b/libglusterfs/src/xlator.c index 1bded6d3d11..4f8b8b21f95 100644 --- a/libglusterfs/src/xlator.c +++ b/libglusterfs/src/xlator.c @@ -647,7 +647,7 @@ out: char* loc_gfid_utoa (loc_t *loc) { - uuid_t gfid; + uuid_t gfid = {0, }; loc_gfid (loc, gfid); return uuid_utoa (gfid); } diff --git a/xlators/cluster/ha/src/ha.c b/xlators/cluster/ha/src/ha.c index 3eccb516b65..742ec46f107 100644 --- a/xlators/cluster/ha/src/ha.c +++ b/xlators/cluster/ha/src/ha.c @@ -2694,7 +2694,12 @@ ha_statfs_cbk (call_frame_t *frame, ha_local_t *local = NULL; ha_private_t *priv = NULL; + GF_ASSERT (this); + local = frame->local; + priv = this->private; + GF_ASSERT (priv); + if (-1 == op_ret) { local->active = (local->active + 1) % priv->child_count; local->tries--; @@ -2711,7 +2716,6 @@ ha_statfs_cbk (call_frame_t *frame, out: loc_wipe (&local->loc); STACK_UNWIND (frame, op_ret, op_errno, buf); - return 0; } diff --git a/xlators/features/changelog/src/changelog.c b/xlators/features/changelog/src/changelog.c index 4ed7cc7f0e9..42292352b72 100644 --- a/xlators/features/changelog/src/changelog.c +++ b/xlators/features/changelog/src/changelog.c @@ -2039,20 +2039,21 @@ init (xlator_t *this) out: if (ret) { - if (this->local_pool) + if (this && this->local_pool) mem_pool_destroy (this->local_pool); - if (priv->cb) { - ret = priv->cb->dtor (this, &priv->cd); - if (ret) - gf_log (this->name, GF_LOG_ERROR, + if (priv) { + if (priv->cb) { + ret = priv->cb->dtor (this, &priv->cd); + if (ret) + gf_log (this->name, GF_LOG_ERROR, "error in cleanup during init()"); + } + GF_FREE (priv->changelog_brick); + GF_FREE (priv->changelog_dir); + if (cond_lock_init) + changelog_pthread_destroy (priv); + GF_FREE (priv); } - GF_FREE (priv->changelog_brick); - GF_FREE (priv->changelog_dir); - if (cond_lock_init) - changelog_pthread_destroy (priv); - - GF_FREE (priv); this->private = NULL; } else this->private = priv; diff --git a/xlators/mgmt/glusterd/src/glusterd-syncop.c b/xlators/mgmt/glusterd/src/glusterd-syncop.c index 23824bd4ec3..1381edbab9d 100644 --- a/xlators/mgmt/glusterd/src/glusterd-syncop.c +++ b/xlators/mgmt/glusterd/src/glusterd-syncop.c @@ -1362,6 +1362,8 @@ gd_unlock_op_phase (glusterd_conf_t *conf, glusterd_op_t op, int *op_ret, struct list_head *peers = NULL; peers = &conf->xaction_peers; + this = THIS; + GF_ASSERT (this); if (!npeers) { ret = 0; @@ -1375,7 +1377,6 @@ gd_unlock_op_phase (glusterd_conf_t *conf, glusterd_op_t op, int *op_ret, goto out; } - this = THIS; synctask_barrier_init((&args)); peer_cnt = 0; if (conf->op_version < GD_OP_VERSION_3_6_0) { diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c index 8515320d894..94949e79111 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.c +++ b/xlators/mgmt/glusterd/src/glusterd-utils.c @@ -8435,9 +8435,10 @@ glusterd_get_local_brickpaths (glusterd_volinfo_t *volinfo, char **pathlist) ret = count; out: - for (i = 0; i < count; i++) { - GF_FREE (path_tokens[i]); - path_tokens[i] = NULL; + if (path_tokens) { + for (i = 0; i < count; i++) { + GF_FREE (path_tokens[i]); + } } GF_FREE (path_tokens); -- cgit