summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKaleb S. KEITHLEY <kkeithle@redhat.com>2014-06-13 11:09:25 -0400
committerVijay Bellur <vbellur@redhat.com>2014-11-25 09:27:36 -0800
commit4ea5b8d2046b9e0bc7f24cdf1b2e72ab8b462c9e (patch)
tree2bd4b9314e30d57b93f0d4302118df65fa277b63
parentc6f27ed5cc9a6fafc6e6f83aff00196bc7a49d38 (diff)
core: fix Ubuntu code audit (cppcheck) results
See also http://review.gluster.org/#/c/7693/, BZ 1091677 AFAICT these are false positives: [geo-replication/src/gsyncd.c:100]: (error) Memory leak: str [geo-replication/src/gsyncd.c:403]: (error) Memory leak: argv [xlators/nfs/server/src/nlm4.c:1201]: (error) Possible null pointer dereference: fde [xlators/cluster/afr/src/afr-self-heal-common.c:138]: (error) Possible null pointer dereference: __ptr [xlators/cluster/afr/src/afr-self-heal-common.c:140]: (error) Possible null pointer dereference: __ptr [xlators/cluster/afr/src/afr-self-heal-common.c:331]: (error) Possible null pointer dereference: __ptr Test program: [extras/test/test-ffop.c:27]: (error) Buffer overrun possible for long command line arguments. [tests/basic/fops-sanity.c:55]: (error) Buffer overrun possible for long command line arguments. the remainder are fixed with this change-set: [cli/src/cli-rpc-ops.c:8883]: (error) Possible null pointer dereference: local [cli/src/cli-rpc-ops.c:8886]: (error) Possible null pointer dereference: local [contrib/uuid/gen_uuid.c:369]: (warning) %ld in format string (no. 2) requires 'long *' but the argument type is 'unsigned long *'. [contrib/uuid/gen_uuid.c:369]: (warning) %ld in format string (no. 3) requires 'long *' but the argument type is 'unsigned long *'. [xlators/cluster/dht/src/dht-rebalance.c:1734]: (error) Possible null pointer dereference: ctx [xlators/cluster/stripe/src/stripe.c:4940]: (error) Possible null pointer dereference: local [xlators/mgmt/glusterd/src/glusterd-geo-rep.c:1718]: (error) Possible null pointer dereference: command [xlators/mgmt/glusterd/src/glusterd-replace-brick.c:942]: (error) Resource leak: file [xlators/mgmt/glusterd/src/glusterd-replace-brick.c:1026]: (error) Resource leak: file [xlators/mgmt/glusterd/src/glusterd-sm.c:249]: (error) Possible null pointer dereference: new_ev_ctx [xlators/mgmt/glusterd/src/glusterd-snapshot.c:6917]: (error) Possible null pointer dereference: volinfo [xlators/mgmt/glusterd/src/glusterd-utils.c:4517]: (error) Possible null pointer dereference: this [xlators/mgmt/glusterd/src/glusterd-utils.c:6662]: (error) Possible null pointer dereference: this [xlators/mgmt/glusterd/src/glusterd-utils.c:7708]: (error) Possible null pointer dereference: this [xlators/mount/fuse/src/fuse-bridge.c:4687]: (error) Uninitialized variable: finh [xlators/mount/fuse/src/fuse-bridge.c:3080]: (error) Possible null pointer dereference: state [xlators/nfs/server/src/nfs-common.c:89]: (error) Dangerous usage of 'volname' (strncpy doesn't always null-terminate it). [xlators/performance/quick-read/src/quick-read.c:586]: (error) Possible null pointer dereference: iobuf Rerunning cppcheck after fixing the above: As before, test program: [extras/test/test-ffop.c:27]: (error) Buffer overrun possible for long command line arguments. [tests/basic/fops-sanity.c:55]: (error) Buffer overrun possible for long command line arguments. As before, false positive: [geo-replication/src/gsyncd.c:100]: (error) Memory leak: str [geo-replication/src/gsyncd.c:403]: (error) Memory leak: argv [xlators/nfs/server/src/nlm4.c:1201]: (error) Possible null pointer dereference: fde [xlators/cluster/afr/src/afr-self-heal-common.c:138]: (error) Possible null pointer dereference: __ptr [xlators/cluster/afr/src/afr-self-heal-common.c:140]: (error) Possible null pointer dereference: __ptr [xlators/cluster/afr/src/afr-self-heal-common.c:331]: (error) Possible null pointer dereference: __ptr False positive after fix: [xlators/performance/quick-read/src/quick-read.c:584]: (error) Possible null pointer dereference: iobuf Change-Id: I20e0e3ac1d600b2f2120b8d8536cd6d9e17023e8 BUG: 1109180 Signed-off-by: Kaleb S. KEITHLEY <kkeithle@redhat.com> Reviewed-on: http://review.gluster.org/8064 Tested-by: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: Vijay Bellur <vbellur@redhat.com>
-rw-r--r--cli/src/cli-rpc-ops.c9
-rw-r--r--contrib/uuid/gen_uuid.c17
-rw-r--r--xlators/cluster/dht/src/dht-rebalance.c2
-rw-r--r--xlators/cluster/stripe/src/stripe.c7
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-geo-rep.c12
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-replace-brick.c15
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-sm.c3
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-snapshot.c14
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-utils.c14
-rw-r--r--xlators/mount/fuse/src/fuse-bridge.c5
-rw-r--r--xlators/nfs/server/src/nfs-common.c6
-rw-r--r--xlators/performance/quick-read/src/quick-read.c11
12 files changed, 64 insertions, 51 deletions
diff --git a/cli/src/cli-rpc-ops.c b/cli/src/cli-rpc-ops.c
index 08f51558506..f86fa63f68d 100644
--- a/cli/src/cli-rpc-ops.c
+++ b/cli/src/cli-rpc-ops.c
@@ -9179,11 +9179,10 @@ gf_cli_snapshot (call_frame_t *frame, xlator_t *this,
if (!frame || !this || !data)
goto out;
- if (frame->local) {
- local = frame->local;
- } else {
+ if (!frame->local)
goto out;
- }
+
+ local = frame->local;
options = data;
@@ -9240,7 +9239,7 @@ gf_cli_snapshot (call_frame_t *frame, xlator_t *this,
ret = 0;
out:
- if (ret && GF_SNAP_OPTION_TYPE_STATUS == type) {
+ if (ret && local && GF_SNAP_OPTION_TYPE_STATUS == type) {
tmp_ret = dict_get_str (local->dict, "op_err_str", &err_str);
if (err_str) {
cli_err ("Snapshot Status : failed: %s", err_str);
diff --git a/contrib/uuid/gen_uuid.c b/contrib/uuid/gen_uuid.c
index e69995ae557..4da0dc69b84 100644
--- a/contrib/uuid/gen_uuid.c
+++ b/contrib/uuid/gen_uuid.c
@@ -44,18 +44,6 @@
#include <windows.h>
#define UUID MYUUID
#endif
-
-#ifdef __APPLE__
-#define PRI_TIME "ld"
-#define PRI_TIME_USEC "d"
-#define SCAN_TIME "lu"
-#else
-#define PRI_TIME "lu"
-#define PRI_TIME_USEC "lu"
-#define SCAN_TIME "ld"
-#endif
-
-
#include <stdio.h>
#ifdef HAVE_UNISTD_H
#include <unistd.h>
@@ -366,7 +354,8 @@ static int get_clock(uint32_t *clock_high, uint32_t *clock_low,
unsigned int cl;
unsigned long tv1, tv2;
int a;
- if (fscanf(state_f, "clock: %04x tv: %" SCAN_TIME " %" SCAN_TIME " adj: %d\n",
+
+ if (fscanf(state_f, "clock: %04x tv: %lu %lu adj: %d\n",
&cl, &tv1, &tv2, &a) == 4) {
clock_seq = cl & 0x3FFF;
last.tv_sec = tv1;
@@ -415,7 +404,7 @@ try_again:
if (state_fd > 0) {
rewind(state_f);
len = fprintf(state_f,
- "clock: %04x tv: %016" PRI_TIME "%08" PRI_TIME_USEC "adj: %08d\n",
+ "clock: %04x tv: %016lu %08lu adj: %08d\n",
clock_seq, last.tv_sec, last.tv_usec, adjustment);
fflush(state_f);
if (ftruncate(state_fd, len) < 0) {
diff --git a/xlators/cluster/dht/src/dht-rebalance.c b/xlators/cluster/dht/src/dht-rebalance.c
index 42c9cde491f..2ca636c4564 100644
--- a/xlators/cluster/dht/src/dht-rebalance.c
+++ b/xlators/cluster/dht/src/dht-rebalance.c
@@ -1974,7 +1974,7 @@ out:
{
status = dict_new ();
gf_defrag_status_get (defrag, status);
- if (ctx->notify)
+ if (ctx && ctx->notify)
ctx->notify (GF_EN_DEFRAG_STATUS, status);
if (status)
dict_unref (status);
diff --git a/xlators/cluster/stripe/src/stripe.c b/xlators/cluster/stripe/src/stripe.c
index 20b889b35e1..7783603c0a5 100644
--- a/xlators/cluster/stripe/src/stripe.c
+++ b/xlators/cluster/stripe/src/stripe.c
@@ -4933,8 +4933,11 @@ out:
if (!count) {
/* all entries are directories */
frame->local = NULL;
- STRIPE_STACK_UNWIND (readdir, frame, local->op_ret,
- local->op_errno, &local->entries, NULL);
+ STRIPE_STACK_UNWIND (readdir, frame,
+ local ? local->op_ret : -1,
+ local ? local->op_errno : EINVAL,
+ local ? &local->entries : NULL,
+ NULL);
gf_dirent_free (&local->entries);
stripe_local_wipe (local);
mem_put (local);
diff --git a/xlators/mgmt/glusterd/src/glusterd-geo-rep.c b/xlators/mgmt/glusterd/src/glusterd-geo-rep.c
index a88729c0e6a..b48ea6f40fd 100644
--- a/xlators/mgmt/glusterd/src/glusterd-geo-rep.c
+++ b/xlators/mgmt/glusterd/src/glusterd-geo-rep.c
@@ -1713,9 +1713,15 @@ glusterd_op_stage_sys_exec (dict_t *dict, char **op_errstr)
out:
if (ret) {
- if (errmsg[0] == '\0')
- snprintf (errmsg, sizeof (errmsg), "%s not found.",
- command);
+ if (errmsg[0] == '\0') {
+ if (command)
+ snprintf (errmsg, sizeof (errmsg),
+ "gsync peer_%s command not found.",
+ command);
+ else
+ snprintf (errmsg, sizeof (errmsg), "%s",
+ "gsync peer command was not specified");
+ }
*op_errstr = gf_strdup (errmsg);
gf_log ("", GF_LOG_ERROR, "%s", errmsg);
}
diff --git a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c
index 5a98d497137..11e15c55f46 100644
--- a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c
+++ b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c
@@ -896,7 +896,7 @@ rb_generate_client_volfile (glusterd_volinfo_t *volinfo,
"%s", strerror (errno));
goto out;
}
- close (fd);
+ sys_close (fd);
file = fopen (filename, "w+");
if (!file) {
@@ -920,12 +920,14 @@ rb_generate_client_volfile (glusterd_volinfo_t *volinfo,
glusterd_auth_get_username (volinfo),
glusterd_auth_get_password (volinfo));
- fclose (file);
GF_FREE (ttype);
ret = 0;
out:
+ if (file)
+ fclose (file);
+
return ret;
}
@@ -969,13 +971,13 @@ rb_generate_dst_brick_volfile (glusterd_volinfo_t *volinfo,
priv->workdir, volinfo->volname,
RB_DSTBRICKVOL_FILENAME);
- fd = creat (filename, S_IRUSR | S_IWUSR);
+ fd = sys_creat (filename, S_IRUSR | S_IWUSR);
if (fd < 0) {
gf_log (this->name, GF_LOG_ERROR,
"%s", strerror (errno));
goto out;
}
- close (fd);
+ sys_close (fd);
file = fopen (filename, "w+");
if (!file) {
@@ -1005,11 +1007,12 @@ rb_generate_dst_brick_volfile (glusterd_volinfo_t *volinfo,
GF_FREE (trans_type);
- fclose (file);
-
ret = 0;
out:
+ if (file)
+ fclose (file);
+
return ret;
}
diff --git a/xlators/mgmt/glusterd/src/glusterd-sm.c b/xlators/mgmt/glusterd/src/glusterd-sm.c
index ca047bd3322..6b30361e3d5 100644
--- a/xlators/mgmt/glusterd/src/glusterd-sm.c
+++ b/xlators/mgmt/glusterd/src/glusterd-sm.c
@@ -246,7 +246,8 @@ glusterd_ac_reverse_probe_begin (glusterd_friend_sm_event_t *event, void *ctx)
out:
if (ret) {
GF_FREE (new_event);
- GF_FREE (new_ev_ctx->hostname);
+ if (new_ev_ctx)
+ GF_FREE (new_ev_ctx->hostname);
GF_FREE (new_ev_ctx);
}
gf_log ("", GF_LOG_DEBUG, "returning with %d", ret);
diff --git a/xlators/mgmt/glusterd/src/glusterd-snapshot.c b/xlators/mgmt/glusterd/src/glusterd-snapshot.c
index 097613184a0..dbe31610d19 100644
--- a/xlators/mgmt/glusterd/src/glusterd-snapshot.c
+++ b/xlators/mgmt/glusterd/src/glusterd-snapshot.c
@@ -7403,6 +7403,20 @@ glusterd_snapshot_restore_postop (dict_t *dict, int32_t op_ret,
goto out;
}
+ ret = dict_get_str (dict, "snapname", &name);
+ if (ret) {
+ gf_log (this->name, GF_LOG_ERROR, "getting the snap "
+ "name failed (volume: %s)", volinfo->volname);
+ goto out;
+ }
+
+ snap = glusterd_find_snap_by_name (name);
+ if (!snap) {
+ gf_log (this->name, GF_LOG_ERROR, "snap %s is not found", name);
+ ret = -1;
+ goto out;
+ }
+
/* On success perform the cleanup operation */
if (0 == op_ret) {
ret = glusterd_snapshot_restore_cleanup (rsp_dict, volinfo,
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index e285a499a5c..6eaa5f6d05a 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -4647,6 +4647,8 @@ glusterd_delete_stale_volume (glusterd_volinfo_t *stale_volinfo,
GF_ASSERT (stale_volinfo);
GF_ASSERT (valid_volinfo);
+ this = THIS;
+ GF_ASSERT (this);
/* Copy snap_volumes list from stale_volinfo to valid_volinfo */
valid_volinfo->snap_count = 0;
@@ -6822,12 +6824,12 @@ glusterd_brick_start (glusterd_volinfo_t *volinfo,
int ret = -1;
xlator_t *this = NULL;
- if ((!brickinfo) || (!volinfo))
- goto out;
-
this = THIS;
GF_ASSERT (this);
+ if ((!brickinfo) || (!volinfo))
+ goto out;
+
if (uuid_is_null (brickinfo->uuid)) {
ret = glusterd_resolve_brick (brickinfo);
if (ret) {
@@ -7821,14 +7823,14 @@ glusterd_brick_stop (glusterd_volinfo_t *volinfo,
xlator_t *this = NULL;
glusterd_conf_t *conf = NULL;
- if ((!brickinfo) || (!volinfo))
- goto out;
-
this = THIS;
GF_ASSERT (this);
conf = this->private;
GF_ASSERT (conf);
+ if ((!brickinfo) || (!volinfo))
+ goto out;
+
if (uuid_is_null (brickinfo->uuid)) {
ret = glusterd_resolve_brick (brickinfo);
if (ret) {
diff --git a/xlators/mount/fuse/src/fuse-bridge.c b/xlators/mount/fuse/src/fuse-bridge.c
index 6290d0a783f..97031e82733 100644
--- a/xlators/mount/fuse/src/fuse-bridge.c
+++ b/xlators/mount/fuse/src/fuse-bridge.c
@@ -3107,6 +3107,8 @@ fuse_setxattr (xlator_t *this, fuse_in_header_t *finh, void *msg)
priv = this->private;
+ GET_STATE (this, finh, state);
+
#ifdef GF_DARWIN_HOST_OS
if (fsi->position) {
gf_log ("glusterfs-fuse", GF_LOG_WARNING,
@@ -3167,7 +3169,6 @@ fuse_setxattr (xlator_t *this, fuse_in_header_t *finh, void *msg)
return;
}
- GET_STATE (this, finh, state);
state->size = fsi->size;
fuse_resolve_inode_init (state, &state->resolve, finh->nodeid);
@@ -4715,7 +4716,7 @@ fuse_thread_proc (void *data)
fuse_private_t *priv = NULL;
ssize_t res = 0;
struct iobuf *iobuf = NULL;
- fuse_in_header_t *finh;
+ fuse_in_header_t *finh = NULL;
struct iovec iov_in[2];
void *msg = NULL;
const size_t msg0_size = sizeof (*finh) + 128;
diff --git a/xlators/nfs/server/src/nfs-common.c b/xlators/nfs/server/src/nfs-common.c
index f74396ee893..f942d37d6e1 100644
--- a/xlators/nfs/server/src/nfs-common.c
+++ b/xlators/nfs/server/src/nfs-common.c
@@ -77,15 +77,15 @@ nfs_xlator_to_xlid (xlator_list_t *cl, xlator_t *xl)
xlator_t *
nfs_mntpath_to_xlator (xlator_list_t *cl, char *path)
{
- char volname[MNTPATHLEN];
+ char *volname = NULL;
char *volptr = NULL;
- size_t pathlen;
+ size_t pathlen;
xlator_t *targetxl = NULL;
if ((!cl) || (!path))
return NULL;
- strncpy (volname, path, MNTPATHLEN);
+ volname = strdupa (path);
pathlen = strlen (volname);
gf_log (GF_NFS, GF_LOG_TRACE, "Subvolume search: %s", path);
if (volname[0] == '/')
diff --git a/xlators/performance/quick-read/src/quick-read.c b/xlators/performance/quick-read/src/quick-read.c
index b8b4c532674..0e4ce71a571 100644
--- a/xlators/performance/quick-read/src/quick-read.c
+++ b/xlators/performance/quick-read/src/quick-read.c
@@ -545,8 +545,6 @@ qr_readv_cached (call_frame_t *frame, qr_inode_t *qr_inode, size_t size,
LOCK (&table->lock);
{
- op_ret = -1;
-
if (!qr_inode->data)
goto unlock;
@@ -582,19 +580,16 @@ qr_readv_cached (call_frame_t *frame, qr_inode_t *qr_inode, size_t size,
unlock:
UNLOCK (&table->lock);
- if (op_ret > 0) {
+ if (op_ret >= 0) {
iov.iov_base = iobuf->ptr;
iov.iov_len = op_ret;
STACK_UNWIND_STRICT (readv, frame, op_ret, 0, &iov, 1,
&buf, iobref, xdata);
}
+ iobuf_unref (iobuf);
- if (iobuf)
- iobuf_unref (iobuf);
-
- if (iobref)
- iobref_unref (iobref);
+ iobref_unref (iobref);
return op_ret;
}