From e7aeab3063ac5645136303278b477d7de35266c0 Mon Sep 17 00:00:00 2001 From: Amar Tumballi Date: Mon, 20 May 2019 11:11:39 +0530 Subject: across: clang-scan: fix NULL dereferencing warnings All these checks are done after analyzing clang-scan report produced by the CI job @ https://build.gluster.org/job/clang-scan updates: bz#1622665 Change-Id: I590305af4ceb779be952974b2a36066ffc4865ca Signed-off-by: Amar Tumballi --- api/src/glfs-handleops.c | 2 +- libglusterfs/src/logging.c | 2 +- rpc/rpc-transport/socket/src/socket.c | 8 +++----- xlators/features/locks/src/common.c | 2 +- xlators/features/marker/src/marker-quota.c | 3 +++ xlators/features/quota/src/quota.c | 10 +++++---- xlators/features/quota/src/quotad-aggregator.c | 5 +++-- xlators/features/sdfs/src/sdfs.c | 3 ++- xlators/features/shard/src/shard.c | 28 +++++++++++++++++--------- xlators/storage/posix/src/posix-helpers.c | 2 +- 10 files changed, 39 insertions(+), 26 deletions(-) diff --git a/api/src/glfs-handleops.c b/api/src/glfs-handleops.c index d4e154526ab..8b4307e1096 100644 --- a/api/src/glfs-handleops.c +++ b/api/src/glfs-handleops.c @@ -2216,7 +2216,7 @@ pub_glfs_h_poll_upcall370(struct glfs *fs, struct glfs_callback_arg *up_arg) ret = pub_glfs_h_poll_upcall(fs, &upcall); if (ret == 0) { up_arg->fs = fs; - if (errno == ENOENT || upcall->event == NULL) { + if ((errno == ENOENT) || !upcall || !upcall->event) { up_arg->reason = GLFS_UPCALL_EVENT_NULL; goto out; } diff --git a/libglusterfs/src/logging.c b/libglusterfs/src/logging.c index f0fb9a097da..32d98e932e6 100644 --- a/libglusterfs/src/logging.c +++ b/libglusterfs/src/logging.c @@ -1873,7 +1873,7 @@ _gf_msg_internal(const char *domain, const char *file, const char *function, if (size == 0) { flush_logged_msg = _gf_true; goto unlock; - } else if ((ctx->log.lru_cur_size + 1) > size) { + } else if (((ctx->log.lru_cur_size + 1) > size) && (first)) { /* If the list is full, flush the lru msg to disk and also * release it after unlock, and ... * */ diff --git a/rpc/rpc-transport/socket/src/socket.c b/rpc/rpc-transport/socket/src/socket.c index fe25f73526b..48f6116c458 100644 --- a/rpc/rpc-transport/socket/src/socket.c +++ b/rpc/rpc-transport/socket/src/socket.c @@ -269,11 +269,9 @@ ssl_do(rpc_transport_t *this, void *buf, size_t len, SSL_trinary_func *func) } r = func(priv->ssl_ssl, buf, len); } else { - /* - * We actually need these functions to get to - * priv->connected == 1. - */ - r = ((SSL_unary_func *)func)(priv->ssl_ssl); + /* This should be treated as error */ + gf_log(this->name, GF_LOG_ERROR, "buffer is empty %s", __func__); + goto out; } switch (SSL_get_error(priv->ssl_ssl, r)) { case SSL_ERROR_NONE: diff --git a/xlators/features/locks/src/common.c b/xlators/features/locks/src/common.c index 24422b494b9..ad1ce6c604d 100644 --- a/xlators/features/locks/src/common.c +++ b/xlators/features/locks/src/common.c @@ -1274,7 +1274,7 @@ pl_local_init(call_frame_t *frame, xlator_t *this, loc_t *loc, fd_t *fd) return -1; } - local->inode = (fd ? inode_ref(fd->inode) : inode_ref(loc->inode)); + local->inode = (loc ? inode_ref(loc->inode) : inode_ref(fd->inode)); frame->local = local; } diff --git a/xlators/features/marker/src/marker-quota.c b/xlators/features/marker/src/marker-quota.c index 31584f5446b..4176ab6a8f1 100644 --- a/xlators/features/marker/src/marker-quota.c +++ b/xlators/features/marker/src/marker-quota.c @@ -2063,6 +2063,9 @@ mq_inspect_directory_xattr(xlator_t *this, quota_inode_ctx_t *ctx, if (ret < 0) goto create_xattr; + if (!contribution) + goto create_xattr; + if (!loc_is_root(loc)) { GET_CONTRI_KEY(this, contri_key, contribution->gfid, keylen); if (keylen < 0) { diff --git a/xlators/features/quota/src/quota.c b/xlators/features/quota/src/quota.c index a0c236d4cf6..1335386a197 100644 --- a/xlators/features/quota/src/quota.c +++ b/xlators/features/quota/src/quota.c @@ -1921,10 +1921,12 @@ quota_writev(call_frame_t *frame, xlator_t *this, fd_t *fd, par_inode = do_quota_check_limit(frame, fd->inode, this, dentry, _gf_false); if (par_inode == NULL) { - /* remove stale entry from inode ctx */ - quota_dentry_del(ctx, dentry->name, dentry->par); - parents--; - fail_count++; + if (ctx) { + /* remove stale entry from inode ctx */ + quota_dentry_del(ctx, dentry->name, dentry->par); + parents--; + fail_count++; + } } else { inode_unref(par_inode); } diff --git a/xlators/features/quota/src/quotad-aggregator.c b/xlators/features/quota/src/quotad-aggregator.c index 5a755c45fda..15b482503e3 100644 --- a/xlators/features/quota/src/quotad-aggregator.c +++ b/xlators/features/quota/src/quotad-aggregator.c @@ -176,8 +176,9 @@ out: } reply: - quotad_aggregator_submit_reply(frame, frame->local, (void *)&cli_rsp, NULL, - 0, NULL, (xdrproc_t)xdr_gf_cli_rsp); + quotad_aggregator_submit_reply(frame, (frame) ? frame->local : NULL, + (void *)&cli_rsp, NULL, 0, NULL, + (xdrproc_t)xdr_gf_cli_rsp); dict_unref(xdata); GF_FREE(cli_rsp.dict.dict_val); diff --git a/xlators/features/sdfs/src/sdfs.c b/xlators/features/sdfs/src/sdfs.c index b3fbc01caf7..3460b475824 100644 --- a/xlators/features/sdfs/src/sdfs.c +++ b/xlators/features/sdfs/src/sdfs.c @@ -177,9 +177,10 @@ sdfs_get_new_frame(call_frame_t *frame, loc_t *loc, call_frame_t **new_frame) ret = 0; err: - if ((ret < 0) && (*new_frame != NULL)) { + if (ret && (*new_frame)) { SDFS_STACK_DESTROY((*new_frame)); *new_frame = NULL; + ret = -1; } return ret; diff --git a/xlators/features/shard/src/shard.c b/xlators/features/shard/src/shard.c index 2249cefc5ab..bb660306212 100644 --- a/xlators/features/shard/src/shard.c +++ b/xlators/features/shard/src/shard.c @@ -886,26 +886,34 @@ int shard_common_inode_write_success_unwind(glusterfs_fop_t fop, call_frame_t *frame, int32_t op_ret) { - shard_local_t *local = NULL; + shard_local_t *local = frame->local; - local = frame->local; + /* the below 3 variables are required because, in SHARD_STACK_UNWIND() + macro, there is a check for local being null. So many static analyzers + backtrace the code with assumption of possible (local == NULL) case, + and complains for below lines. By handling it like below, we overcome + the warnings */ + + struct iatt *prebuf = ((local) ? &local->prebuf : NULL); + struct iatt *postbuf = ((local) ? &local->postbuf : NULL); + dict_t *xattr_rsp = ((local) ? local->xattr_rsp : NULL); switch (fop) { case GF_FOP_WRITE: - SHARD_STACK_UNWIND(writev, frame, op_ret, 0, &local->prebuf, - &local->postbuf, local->xattr_rsp); + SHARD_STACK_UNWIND(writev, frame, op_ret, 0, prebuf, postbuf, + xattr_rsp); break; case GF_FOP_FALLOCATE: - SHARD_STACK_UNWIND(fallocate, frame, op_ret, 0, &local->prebuf, - &local->postbuf, local->xattr_rsp); + SHARD_STACK_UNWIND(fallocate, frame, op_ret, 0, prebuf, postbuf, + xattr_rsp); break; case GF_FOP_ZEROFILL: - SHARD_STACK_UNWIND(zerofill, frame, op_ret, 0, &local->prebuf, - &local->postbuf, local->xattr_rsp); + SHARD_STACK_UNWIND(zerofill, frame, op_ret, 0, prebuf, postbuf, + xattr_rsp); break; case GF_FOP_DISCARD: - SHARD_STACK_UNWIND(discard, frame, op_ret, 0, &local->prebuf, - &local->postbuf, local->xattr_rsp); + SHARD_STACK_UNWIND(discard, frame, op_ret, 0, prebuf, postbuf, + xattr_rsp); break; default: gf_msg(THIS->name, GF_LOG_WARNING, 0, SHARD_MSG_INVALID_FOP, diff --git a/xlators/storage/posix/src/posix-helpers.c b/xlators/storage/posix/src/posix-helpers.c index 7296f698db0..bab79f42c2f 100644 --- a/xlators/storage/posix/src/posix-helpers.c +++ b/xlators/storage/posix/src/posix-helpers.c @@ -2416,7 +2416,7 @@ posix_fsyncer_process(xlator_t *this, call_stub_t *stub, gf_boolean_t do_fsync) return; } - if (do_fsync) { + if (do_fsync && pfd) { if (stub->args.datasync) ret = sys_fdatasync(pfd->fd); else -- cgit