diff options
| author | Kaleb S. KEITHLEY <kkeithle@redhat.com> | 2014-04-28 14:25:09 -0400 | 
|---|---|---|
| committer | Kaleb KEITHLEY <kkeithle@redhat.com> | 2014-05-25 03:52:01 -0700 | 
| commit | d5e317718f4137431c3996ac5f38e9226620e760 (patch) | |
| tree | 390d5d4efe1c9a877cedfa353ba1095a1642b33a | |
| parent | 4f8f96c62b21185f27d8e76912a808af80e22608 (diff) | |
core: fix Ubuntu code audit (cppcheck) results
These block inclusion in Ubuntu Main repo.
AFAICT these are false positives:
[rpc/rpc-transport/rdma/src/rdma.c:3074]: (error) Address of local auto-variable assigned to a function parameter.
[xlators/features/marker/utils/src/gsyncd.c:99]: (error) Memory leak: str
[xlators/features/marker/utils/src/gsyncd.c:354]: (error) Memory leak: argv
[xlators/nfs/server/src/nlm4.c:1176]: (error) Possible null pointer dereference: fde
The remainder are fixed with this change-set:
[api/src/glfs-fops.c:700]: (error) Possible null pointer dereference: gio
[api/src/glfs-fops.c:702]: (error) Possible null pointer dereference: frame
[xlators/cluster/afr/src/afr-inode-write.c:375]: (error) Possible null pointer dereference: frame
[xlators/cluster/afr/src/afr-self-heal-common.c:1522]: (error) Possible null pointer dereference: local
[xlators/cluster/dht/src/dht-rebalance.c:1574]: (error) Possible null pointer dereference: ctx
[xlators/cluster/stripe/src/stripe.c:4407]: (error) Possible null pointer dereference: local
[xlators/mgmt/glusterd/src/glusterd-mountbroker.c:675]: (error) Possible null pointer dereference: cookieswitch
[xlators/mgmt/glusterd/src/glusterd-mountbroker.c:677]: (error) Possible null pointer dereference: cookieswitch
[xlators/mgmt/glusterd/src/glusterd-replace-brick.c:924]: (error) Resource leak: file
[xlators/mgmt/glusterd/src/glusterd-replace-brick.c:1008]: (error) Resource leak: file
[xlators/mgmt/glusterd/src/glusterd-sm.c:248]: (error) Possible null pointer dereference: new_ev_ctx
[xlators/mgmt/glusterd/src/glusterd-store.c:1250]: (error) Possible null pointer dereference: handle
[xlators/mgmt/glusterd/src/glusterd-utils.c:4272]: (error) Possible null pointer dereference: this
[xlators/mgmt/glusterd/src/glusterd-utils.c:5113]: (error) Possible null pointer dereference: this
[xlators/mount/fuse/src/fuse-bridge.c:4432]: (error) Uninitialized variable: finh
[xlators/mount/fuse/src/fuse-bridge.c:2927]: (error) Possible null pointer dereference: state
[xlators/mount/fuse/src/fuse-bridge.c:3226]: (error) Possible null pointer dereference: state
[xlators/storage/bd_map/src/bd_map.c:1504]: (error) Possible null pointer dereference: bd_fd
[xlators/storage/bd_map/src/bd_map.c:1728]: (error) Possible null pointer dereference: n_entry
[xlators/storage/bd_map/src/bd_map.c:1741]: (error) Possible null pointer dereference: n_entry
[xlators/performance/quick-read/src/quick-read.c:585]: (error) Possible null pointer dereference: iobuf
rerunning cppcheck --force afterwards:
Test code, don't care:
[extras/test/test-ffop.c:27]: (error) Buffer overrun possible for long command line arguments.
False positive after fix
[xlators/cluster/stripe/src/stripe.c:4407]: (error) Possible null pointer dereference: local
Still false positive:
[xlators/features/marker/utils/src/gsyncd.c:354]: (error) Memory leak: argv
[xlators/nfs/server/src/nlm4.c:1176]: (error) Possible null pointer dereference: fde
Not built, don't care:
[xlators/cluster/ha/src/ha.c:2699]: (error) Possible null pointer dereference: priv
Change-Id: I1fb849e9c042d3a3701cb05121d413e58e73d505
BUG: 1086460
Signed-off-by: Kaleb S. KEITHLEY <kkeithle@redhat.com>
Reviewed-on: http://review.gluster.org/7583
Reviewed-by: Vijay Bellur <vbellur@redhat.com>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
| -rw-r--r-- | api/src/glfs-fops.c | 6 | ||||
| -rw-r--r-- | xlators/cluster/afr/src/afr-self-heal-common.c | 4 | ||||
| -rw-r--r-- | xlators/cluster/afr/src/afr.h | 10 | ||||
| -rw-r--r-- | xlators/cluster/dht/src/dht-common.c | 4 | ||||
| -rw-r--r-- | xlators/cluster/dht/src/dht-rebalance.c | 2 | ||||
| -rw-r--r-- | xlators/cluster/stripe/src/stripe.c | 13 | ||||
| -rw-r--r-- | xlators/features/marker/utils/src/gsyncd.c | 2 | ||||
| -rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-mountbroker.c | 2 | ||||
| -rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-replace-brick.c | 6 | ||||
| -rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-sm.c | 3 | ||||
| -rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-store.c | 4 | ||||
| -rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-utils.c | 12 | ||||
| -rw-r--r-- | xlators/mount/fuse/src/fuse-bridge.c | 9 | ||||
| -rw-r--r-- | xlators/nfs/server/src/nfs-common.c | 11 | ||||
| -rw-r--r-- | xlators/performance/quick-read/src/quick-read.c | 13 | ||||
| -rw-r--r-- | xlators/storage/bd_map/src/bd_map.c | 9 | 
16 files changed, 55 insertions, 55 deletions
diff --git a/api/src/glfs-fops.c b/api/src/glfs-fops.c index bda7e9e467e..61843c8a857 100644 --- a/api/src/glfs-fops.c +++ b/api/src/glfs-fops.c @@ -697,9 +697,11 @@ glfs_preadv_async (struct glfs_fd *glfd, const struct iovec *iovec, int count,  out:  	if (ret) { -		GF_FREE (gio->iov); +                if (gio) +		        GF_FREE (gio->iov);  		GF_FREE (gio); -		STACK_DESTROY (frame->root); +                if (frame) +		        STACK_DESTROY (frame->root);  		glfs_subvol_done (fs, subvol);  	} diff --git a/xlators/cluster/afr/src/afr-self-heal-common.c b/xlators/cluster/afr/src/afr-self-heal-common.c index 600991d5058..c6ce7dc5f39 100644 --- a/xlators/cluster/afr/src/afr-self-heal-common.c +++ b/xlators/cluster/afr/src/afr-self-heal-common.c @@ -1508,6 +1508,8 @@ afr_sh_remove_entry_cbk (call_frame_t *frame, xlator_t *this, int child,          afr_self_heal_t *sh = NULL;          local = frame->local; +        GF_ASSERT (local); +          sh = &local->self_heal;          GF_ASSERT (sh->post_remove_call); @@ -1567,7 +1569,7 @@ afr_sh_call_entry_expunge_remove (call_frame_t *frame, xlator_t *this,          return;  out:          gf_log (this->name, GF_LOG_ERROR, "Expunge of %s failed, reason: %s", -                local->loc.path, strerror (op_errno)); +                local ? local->loc.path : "<unknown>" , strerror (op_errno));          expunge_done (frame, this, child_index, -1, op_errno);  } diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h index 978339017a2..84e8907f993 100644 --- a/xlators/cluster/afr/src/afr.h +++ b/xlators/cluster/afr/src/afr.h @@ -892,10 +892,12 @@ afr_launch_openfd_self_heal (call_frame_t *frame, xlator_t *this, fd_t *fd);          do {                                                    \                  afr_local_t *__local = NULL;                    \                  xlator_t    *__this = NULL;                     \ -                __local = frame->local;                         \ -                __this = frame->this;                           \ -                frame->local = NULL;                            \ -                STACK_DESTROY (frame->root);                    \ +                if (frame) {                                    \ +                        __local = frame->local;                 \ +                        __this = frame->this;                   \ +                        frame->local = NULL;                    \ +                        STACK_DESTROY (frame->root);            \ +                }                                               \                  if (__local) {                                  \                          afr_local_cleanup (__local, __this);    \                          mem_put (__local);                      \ diff --git a/xlators/cluster/dht/src/dht-common.c b/xlators/cluster/dht/src/dht-common.c index 29bfd21606b..0234bbbd409 100644 --- a/xlators/cluster/dht/src/dht-common.c +++ b/xlators/cluster/dht/src/dht-common.c @@ -2062,10 +2062,8 @@ dht_getxattr_get_real_filename_cbk (call_frame_t *frame, void *cookie,  {          int             this_call_cnt = 0;          dht_local_t     *local = NULL; -        dht_conf_t      *conf = NULL; -        conf = this->private;          local = frame->local;  	if (op_ret != -1) { @@ -2092,7 +2090,6 @@ int  dht_getxattr_get_real_filename (call_frame_t *frame, xlator_t *this,  				loc_t *loc, const char *key, dict_t *xdata)  { -	dht_conf_t      *conf = NULL;  	dht_local_t     *local = NULL;  	int              i = 0;  	dht_layout_t    *layout = NULL; @@ -2100,7 +2097,6 @@ dht_getxattr_get_real_filename (call_frame_t *frame, xlator_t *this,  	xlator_t        *subvol = NULL; -        conf   = this->private;  	local = frame->local;  	layout = local->layout; diff --git a/xlators/cluster/dht/src/dht-rebalance.c b/xlators/cluster/dht/src/dht-rebalance.c index d212f6eae25..c181bebd131 100644 --- a/xlators/cluster/dht/src/dht-rebalance.c +++ b/xlators/cluster/dht/src/dht-rebalance.c @@ -1571,7 +1571,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 b3e87d31b5a..bdea45f3ed6 100644 --- a/xlators/cluster/stripe/src/stripe.c +++ b/xlators/cluster/stripe/src/stripe.c @@ -4402,10 +4402,15 @@ unlock:  out:          if (!count) {                  /* all entries are directories */ -                frame->local = NULL; -                STRIPE_STACK_UNWIND (readdir, frame, local->op_ret, -                                     local->op_errno, &local->entries, NULL); -                gf_dirent_free (&local->entries); +                if (frame) +                        frame->local = NULL; +                STRIPE_STACK_UNWIND (readdir, frame, +                                     local ? local->op_ret : -1, +                                     local ? local->op_errno : EINVAL, +                                     local ? &local->entries : NULL, +                                     NULL); +                if (local) +                        gf_dirent_free (&local->entries);                  stripe_local_wipe (local);                  mem_put (local);          } diff --git a/xlators/features/marker/utils/src/gsyncd.c b/xlators/features/marker/utils/src/gsyncd.c index 9c4a5bdffb3..1f9eb2fa958 100644 --- a/xlators/features/marker/utils/src/gsyncd.c +++ b/xlators/features/marker/utils/src/gsyncd.c @@ -70,7 +70,7 @@ str2argv (char *str, char ***argv)          int ret         = 0;          assert (str); -        str = strdup (str); +        str = gf_strdup (str);          if (!str)                  return -1; diff --git a/xlators/mgmt/glusterd/src/glusterd-mountbroker.c b/xlators/mgmt/glusterd/src/glusterd-mountbroker.c index 0d67d130360..c96804a80b4 100644 --- a/xlators/mgmt/glusterd/src/glusterd-mountbroker.c +++ b/xlators/mgmt/glusterd/src/glusterd-mountbroker.c @@ -671,7 +671,7 @@ glusterd_do_mount (char *label, dict_t *argdict, char **path, int *op_errno)                  ret = -1;                  gf_log ("", GF_LOG_WARNING, "unsuccessful mount request (%s)",                          strerror (*op_errno)); -                if (mtptemp) { +                if (mtptemp && cookieswitch) {                          *cookieswitch = '/';                          unlink (mtptemp);                          *cookieswitch = '\0'; diff --git a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c index 5f299a7eff5..da02edd3674 100644 --- a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c +++ b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c @@ -891,7 +891,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) { @@ -906,6 +906,7 @@ rb_generate_client_volfile (glusterd_volinfo_t *volinfo,  	ttype = glusterd_get_trans_type_rb (volinfo->transport_type);  	if (NULL == ttype){  		ret = -1; +                fclose (file);  		goto out;  	} @@ -970,7 +971,7 @@ rb_generate_dst_brick_volfile (glusterd_volinfo_t *volinfo,                          "%s", strerror (errno));                  goto out;          } -        close (fd); +        sys_close (fd);          file = fopen (filename, "w+");          if (!file) { @@ -983,6 +984,7 @@ rb_generate_dst_brick_volfile (glusterd_volinfo_t *volinfo,  	trans_type = glusterd_get_trans_type_rb (volinfo->transport_type);  	if (NULL == trans_type){  		ret = -1; +                fclose (file);  		goto out;  	} diff --git a/xlators/mgmt/glusterd/src/glusterd-sm.c b/xlators/mgmt/glusterd/src/glusterd-sm.c index c2bf493004a..8d9c5215fa9 100644 --- a/xlators/mgmt/glusterd/src/glusterd-sm.c +++ b/xlators/mgmt/glusterd/src/glusterd-sm.c @@ -245,7 +245,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-store.c b/xlators/mgmt/glusterd/src/glusterd-store.c index 1790c5aec24..023fdc62f65 100644 --- a/xlators/mgmt/glusterd/src/glusterd-store.c +++ b/xlators/mgmt/glusterd/src/glusterd-store.c @@ -1247,10 +1247,10 @@ glusterd_store_global_info (xlator_t *this)          ret = gf_store_rename_tmppath (handle);  out: -        if (ret && (handle->fd > 0)) +        if (ret && handle && (handle->fd > 0))                  gf_store_unlink_tmppath (handle); -        if (handle->fd > 0) { +        if (handle && handle->fd > 0) {                  close (handle->fd);                  handle->fd = 0;          } diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c index c336337e866..6706cb6f36c 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.c +++ b/xlators/mgmt/glusterd/src/glusterd-utils.c @@ -4240,14 +4240,14 @@ glusterd_brick_start (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) { @@ -5074,14 +5074,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 e9c5383e65a..e531970e5c4 100644 --- a/xlators/mount/fuse/src/fuse-bridge.c +++ b/xlators/mount/fuse/src/fuse-bridge.c @@ -2919,6 +2919,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, @@ -2977,7 +2979,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); @@ -3210,6 +3211,8 @@ fuse_getxattr (xlator_t *this, fuse_in_header_t *finh, void *msg)          priv = this->private; +        GET_STATE (this, finh, state); +  #ifdef GF_DARWIN_HOST_OS          if (fgxi->position) {                  /* position can be used only for @@ -3247,8 +3250,6 @@ fuse_getxattr (xlator_t *this, fuse_in_header_t *finh, void *msg)                  }          } -        GET_STATE (this, finh, state); -          fuse_resolve_inode_init (state, &state->resolve, finh->nodeid);          rv = fuse_flip_xattr_ns (priv, name, &newkey); @@ -4425,7 +4426,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 781e557ba8d..07d5382a861 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            *volptr = NULL; +        char           *volname = NULL; +        char           *volptr = NULL;          size_t          pathlen; -        xlator_t        *targetxl = NULL; +        xlator_t       *targetxl = NULL;          if ((!cl) || (!path))                  return NULL; -        strcpy (volname, path); +        volname = strdupa (path);          pathlen = strlen (volname);          gf_log (GF_NFS, GF_LOG_TRACE, "Subvolume search: %s", path);          if (volname[0] == '/') @@ -101,12 +101,9 @@ nfs_mntpath_to_xlator (xlator_list_t *cl, char *path)                          targetxl = cl->xlator;                          break;                  } -                  cl = cl->next;          } -          return targetxl; -  } diff --git a/xlators/performance/quick-read/src/quick-read.c b/xlators/performance/quick-read/src/quick-read.c index 445ea8658ea..55c5afcf877 100644 --- a/xlators/performance/quick-read/src/quick-read.c +++ b/xlators/performance/quick-read/src/quick-read.c @@ -543,8 +543,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; @@ -565,7 +563,6 @@ qr_readv_cached (call_frame_t *frame, qr_inode_t *qr_inode, size_t size,  		iobref = iobref_new ();  		if (!iobref) {  			op_ret = -1; -			iobuf_unref (iobuf);  			goto unlock;  		} @@ -581,19 +578,15 @@ 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;  } diff --git a/xlators/storage/bd_map/src/bd_map.c b/xlators/storage/bd_map/src/bd_map.c index 9c8f69c6488..fcedaedb3ba 100644 --- a/xlators/storage/bd_map/src/bd_map.c +++ b/xlators/storage/bd_map/src/bd_map.c @@ -1500,11 +1500,10 @@ bd_opendir (call_frame_t *frame, xlator_t *this,          op_ret = 0;  out: -        if (op_ret == -1) { +        if (op_ret == -1 && bd_fd) {                  BD_PUT_ENTRY (priv, bd_fd->p_entry); -                if (bd_fd) -                        GF_FREE (bd_fd);          } +        GF_FREE (bd_fd);          STACK_UNWIND_STRICT (opendir, frame, op_ret, op_errno, fd, NULL);          return 0; @@ -1725,7 +1724,7 @@ __bd_fill_readdir (pthread_rwlock_t *bd_lock, bd_fd_t *bd_fd, off_t off,          BD_RD_LOCK (bd_lock); -        bdentry = list_entry ((&bd_fd->p_entry->child)->next, typeof(*n_entry), +        bdentry = list_entry ((&bd_fd->p_entry->child)->next, bd_entry_t,                          child);          if (off) { @@ -1738,7 +1737,7 @@ __bd_fill_readdir (pthread_rwlock_t *bd_lock, bd_fd_t *bd_fd, off_t off,                  }          } else                  bd_fd->entry = list_entry ((&bdentry->sibling), -                                typeof(*n_entry), sibling); +                                            bd_entry_t, sibling);          while (filled <= size) {                  cur_entry = bd_fd->entry;  | 
