clang: Fix various missing checks for empty list

When using list_for_each_entry(_safe) functions, care needs
to be taken that the list passed in are not empty, as these
functions are not empty list safe.

clag scan reported various points where this this pattern
could be caught, and this patch fixes the same.

Additionally the following changes are present in this patch,
- Added an explicit op_ret setting in error case in the
macro MAKE_INODE_HANDLE to address another clang issue reported
- Minor refactoring of some functions in quota code, to address
possible allocation failures in certain functions (which in turn
cause possible empty lists to be passed around)

Change-Id: I1e761a8d218708f714effb56fa643df2a3ea2cc7
Updates: bz#1622665
Signed-off-by: ShyamsundarR <srangana@redhat.com>

11 files changed, 176 insertions, 137 deletions

diff --git a/api/src/glfs-fops.c b/api/src/glfs-fops.c
index f59990aed1..a534697fc1 100644
--- a/api/src/glfs-fops.c
+++ b/api/src/glfs-fops.c
@@ -5314,9 +5314,7 @@ glfs_recall_lease_fd(struct glfs *fs, struct gf_upcall *up_data)
     inode_t *inode = NULL;
     struct glfs_fd *glfd = NULL;
     struct glfs_fd *tmp = NULL;
-    struct list_head glfd_list = {
-        0,
-    };
+    struct list_head glfd_list;
     fd_t *fd = NULL;
     uint64_t value = 0;
     struct glfs_lease lease = {
@@ -5365,22 +5363,24 @@ glfs_recall_lease_fd(struct glfs *fs, struct gf_upcall *up_data)
     }
     UNLOCK(&inode->lock);
 
-    list_for_each_entry_safe(glfd, tmp, &glfd_list, list)
-    {
-        LOCK(&glfd->lock);
+    if (!list_empty(&glfd_list)) {
+        list_for_each_entry_safe(glfd, tmp, &glfd_list, list)
         {
-            if (glfd->state != GLFD_CLOSE) {
-                gf_msg_trace(THIS->name, 0,
-                             "glfd (%p) has held lease, "
-                             "calling recall cbk",
-                             glfd);
-                glfd->cbk(lease, glfd->cookie);
+            LOCK(&glfd->lock);
+            {
+                if (glfd->state != GLFD_CLOSE) {
+                    gf_msg_trace(THIS->name, 0,
+                                 "glfd (%p) has held lease, "
+                                 "calling recall cbk",
+                                 glfd);
+                    glfd->cbk(lease, glfd->cookie);
+                }
             }
-        }
-        UNLOCK(&glfd->lock);
+            UNLOCK(&glfd->lock);
 
-        list_del_init(&glfd->list);
-        GF_REF_PUT(glfd);
+            list_del_init(&glfd->list);
+            GF_REF_PUT(glfd);
+        }
     }
 
 out:
diff --git a/rpc/rpc-lib/src/rpcsvc.c b/rpc/rpc-lib/src/rpcsvc.c
index 0a80bb179c..273443d561 100644
--- a/rpc/rpc-lib/src/rpcsvc.c
+++ b/rpc/rpc-lib/src/rpcsvc.c
@@ -2176,9 +2176,7 @@ rpcsvc_request_handler(void *arg)
     rpcsvc_actor_t *actor = NULL;
     gf_boolean_t done = _gf_false;
     int ret = 0;
-    struct list_head tmp_list = {
-        0,
-    };
+    struct list_head tmp_list;
 
     queue = arg;
     program = queue->program;
diff --git a/xlators/features/locks/src/clear.c b/xlators/features/locks/src/clear.c
index fbd17942c6..578d6d1866 100644
--- a/xlators/features/locks/src/clear.c
+++ b/xlators/features/locks/src/clear.c
@@ -254,14 +254,16 @@ blkd:
     }
     pthread_mutex_unlock(&pl_inode->mutex);
 
-    list_for_each_entry_safe(ilock, tmp, &released, blocked_locks)
-    {
-        list_del_init(&ilock->blocked_locks);
-        pl_trace_out(this, ilock->frame, NULL, NULL, F_SETLKW,
-                     &ilock->user_flock, -1, EAGAIN, ilock->volume);
-        STACK_UNWIND_STRICT(inodelk, ilock->frame, -1, EAGAIN, NULL);
-        // No need to take lock as the locks are only in one list
-        __pl_inodelk_unref(ilock);
+    if (!list_empty(&released)) {
+        list_for_each_entry_safe(ilock, tmp, &released, blocked_locks)
+        {
+            list_del_init(&ilock->blocked_locks);
+            pl_trace_out(this, ilock->frame, NULL, NULL, F_SETLKW,
+                         &ilock->user_flock, -1, EAGAIN, ilock->volume);
+            STACK_UNWIND_STRICT(inodelk, ilock->frame, -1, EAGAIN, NULL);
+            // No need to take lock as the locks are only in one list
+            __pl_inodelk_unref(ilock);
+        }
     }
 
     if (!(args->kind & CLRLK_GRANTED)) {
@@ -357,15 +359,17 @@ blkd:
     }
     pthread_mutex_unlock(&pl_inode->mutex);
 
-    list_for_each_entry_safe(elock, tmp, &released, blocked_locks)
-    {
-        list_del_init(&elock->blocked_locks);
-        entrylk_trace_out(this, elock->frame, elock->volume, NULL, NULL,
-                          elock->basename, ENTRYLK_LOCK, elock->type, -1,
-                          EAGAIN);
-        STACK_UNWIND_STRICT(entrylk, elock->frame, -1, EAGAIN, NULL);
+    if (!list_empty(&released)) {
+        list_for_each_entry_safe(elock, tmp, &released, blocked_locks)
+        {
+            list_del_init(&elock->blocked_locks);
+            entrylk_trace_out(this, elock->frame, elock->volume, NULL, NULL,
+                              elock->basename, ENTRYLK_LOCK, elock->type, -1,
+                              EAGAIN);
+            STACK_UNWIND_STRICT(entrylk, elock->frame, -1, EAGAIN, NULL);
 
-        __pl_entrylk_unref(elock);
+            __pl_entrylk_unref(elock);
+        }
     }
 
     if (!(args->kind & CLRLK_GRANTED)) {
diff --git a/xlators/features/locks/src/entrylk.c b/xlators/features/locks/src/entrylk.c
index d4135542cc..8aa1283160 100644
--- a/xlators/features/locks/src/entrylk.c
+++ b/xlators/features/locks/src/entrylk.c
@@ -1072,32 +1072,36 @@ pl_entrylk_client_cleanup(xlator_t *this, pl_ctx_t *ctx)
     }
     pthread_mutex_unlock(&ctx->lock);
 
-    list_for_each_entry_safe(l, tmp, &unwind, client_list)
-    {
-        list_del_init(&l->client_list);
+    if (!list_empty(&unwind)) {
+        list_for_each_entry_safe(l, tmp, &unwind, client_list)
+        {
+            list_del_init(&l->client_list);
 
-        if (l->frame)
-            STACK_UNWIND_STRICT(entrylk, l->frame, -1, EAGAIN, NULL);
-        list_add_tail(&l->client_list, &released);
+            if (l->frame)
+                STACK_UNWIND_STRICT(entrylk, l->frame, -1, EAGAIN, NULL);
+            list_add_tail(&l->client_list, &released);
+        }
     }
 
-    list_for_each_entry_safe(l, tmp, &released, client_list)
-    {
-        list_del_init(&l->client_list);
+    if (!list_empty(&released)) {
+        list_for_each_entry_safe(l, tmp, &released, client_list)
+        {
+            list_del_init(&l->client_list);
 
-        pinode = l->pinode;
+            pinode = l->pinode;
 
-        dom = get_domain(pinode, l->volume);
+            dom = get_domain(pinode, l->volume);
 
-        grant_blocked_entry_locks(this, pinode, dom, &now, pcontend);
+            grant_blocked_entry_locks(this, pinode, dom, &now, pcontend);
 
-        pthread_mutex_lock(&pinode->mutex);
-        {
-            __pl_entrylk_unref(l);
-        }
-        pthread_mutex_unlock(&pinode->mutex);
+            pthread_mutex_lock(&pinode->mutex);
+            {
+                __pl_entrylk_unref(l);
+            }
+            pthread_mutex_unlock(&pinode->mutex);
 
-        inode_unref(pinode->inode);
+            inode_unref(pinode->inode);
+        }
     }
 
     if (pcontend != NULL) {
diff --git a/xlators/features/locks/src/inodelk.c b/xlators/features/locks/src/inodelk.c
index 19b6f739f2..98367831dc 100644
--- a/xlators/features/locks/src/inodelk.c
+++ b/xlators/features/locks/src/inodelk.c
@@ -691,31 +691,35 @@ pl_inodelk_client_cleanup(xlator_t *this, pl_ctx_t *ctx)
     }
     pthread_mutex_unlock(&ctx->lock);
 
-    list_for_each_entry_safe(l, tmp, &unwind, client_list)
-    {
-        list_del_init(&l->client_list);
+    if (!list_empty(&unwind)) {
+        list_for_each_entry_safe(l, tmp, &unwind, client_list)
+        {
+            list_del_init(&l->client_list);
 
-        if (l->frame)
-            STACK_UNWIND_STRICT(inodelk, l->frame, -1, EAGAIN, NULL);
-        list_add_tail(&l->client_list, &released);
+            if (l->frame)
+                STACK_UNWIND_STRICT(inodelk, l->frame, -1, EAGAIN, NULL);
+            list_add_tail(&l->client_list, &released);
+        }
     }
 
-    list_for_each_entry_safe(l, tmp, &released, client_list)
-    {
-        list_del_init(&l->client_list);
+    if (!list_empty(&released)) {
+        list_for_each_entry_safe(l, tmp, &released, client_list)
+        {
+            list_del_init(&l->client_list);
 
-        pl_inode = l->pl_inode;
+            pl_inode = l->pl_inode;
 
-        dom = get_domain(pl_inode, l->volume);
+            dom = get_domain(pl_inode, l->volume);
 
-        grant_blocked_inode_locks(this, pl_inode, dom, &now, pcontend);
+            grant_blocked_inode_locks(this, pl_inode, dom, &now, pcontend);
 
-        pthread_mutex_lock(&pl_inode->mutex);
-        {
-            __pl_inodelk_unref(l);
+            pthread_mutex_lock(&pl_inode->mutex);
+            {
+                __pl_inodelk_unref(l);
+            }
+            pthread_mutex_unlock(&pl_inode->mutex);
+            inode_unref(pl_inode->inode);
         }
-        pthread_mutex_unlock(&pl_inode->mutex);
-        inode_unref(pl_inode->inode);
     }
 
     if (pcontend != NULL) {
diff --git a/xlators/features/locks/src/posix.c b/xlators/features/locks/src/posix.c
index ce6b6f9fbf..2d7fd112c8 100644
--- a/xlators/features/locks/src/posix.c
+++ b/xlators/features/locks/src/posix.c
@@ -2562,25 +2562,33 @@ pl_forget(xlator_t *this, inode_t *inode)
     }
     pthread_mutex_unlock(&pl_inode->mutex);
 
-    list_for_each_entry_safe(ext_l, ext_tmp, &posixlks_released, list)
-    {
-        STACK_UNWIND_STRICT(lk, ext_l->frame, -1, 0, &ext_l->user_flock, NULL);
-        __destroy_lock(ext_l);
+    if (!list_empty(&posixlks_released)) {
+        list_for_each_entry_safe(ext_l, ext_tmp, &posixlks_released, list)
+        {
+            STACK_UNWIND_STRICT(lk, ext_l->frame, -1, 0, &ext_l->user_flock,
+                                NULL);
+            __destroy_lock(ext_l);
+        }
     }
 
-    list_for_each_entry_safe(ino_l, ino_tmp, &inodelks_released, blocked_locks)
-    {
-        STACK_UNWIND_STRICT(inodelk, ino_l->frame, -1, 0, NULL);
-        __pl_inodelk_unref(ino_l);
+    if (!list_empty(&inodelks_released)) {
+        list_for_each_entry_safe(ino_l, ino_tmp, &inodelks_released,
+                                 blocked_locks)
+        {
+            STACK_UNWIND_STRICT(inodelk, ino_l->frame, -1, 0, NULL);
+            __pl_inodelk_unref(ino_l);
+        }
     }
 
-    list_for_each_entry_safe(entry_l, entry_tmp, &entrylks_released,
-                             blocked_locks)
-    {
-        STACK_UNWIND_STRICT(entrylk, entry_l->frame, -1, 0, NULL);
-        GF_FREE((char *)entry_l->basename);
-        GF_FREE(entry_l->connection_id);
-        GF_FREE(entry_l);
+    if (!list_empty(&entrylks_released)) {
+        list_for_each_entry_safe(entry_l, entry_tmp, &entrylks_released,
+                                 blocked_locks)
+        {
+            STACK_UNWIND_STRICT(entrylk, entry_l->frame, -1, 0, NULL);
+            GF_FREE((char *)entry_l->basename);
+            GF_FREE(entry_l->connection_id);
+            GF_FREE(entry_l);
+        }
     }
 
     pthread_mutex_destroy(&pl_inode->mutex);
diff --git a/xlators/features/quota/src/quota.c b/xlators/features/quota/src/quota.c
index 84df5c3caf..418edfc094 100644
--- a/xlators/features/quota/src/quota.c
+++ b/xlators/features/quota/src/quota.c
@@ -674,39 +674,43 @@ quota_timeout(struct timeval *tv, int32_t timeout)
 
 /* Return: 1 if new entry added
  *         0 no entry added
+ *        -1 on errors
  */
 static int32_t
 quota_add_parent(struct list_head *list, char *name, uuid_t pgfid)
 {
     quota_dentry_t *entry = NULL;
     gf_boolean_t found = _gf_false;
+    int ret = 0;
 
-    if (list == NULL) {
-        goto out;
-    }
-
-    list_for_each_entry(entry, list, next)
-    {
-        if (gf_uuid_compare(pgfid, entry->par) == 0) {
-            found = _gf_true;
-            goto out;
+    if (!list_empty(list)) {
+        list_for_each_entry(entry, list, next)
+        {
+            if (gf_uuid_compare(pgfid, entry->par) == 0) {
+                found = _gf_true;
+                goto out;
+            }
         }
     }
 
     entry = __quota_dentry_new(NULL, name, pgfid);
     if (entry)
         list_add_tail(&entry->next, list);
+    else
+        ret = -1;
 
 out:
     if (found)
         return 0;
-    else
+    else if (ret == 0)
         return 1;
+    else
+        return -1;
 }
 
 /* This function iterates the parent list in inode
  * context and add unique parent to the list
- * Returns number of dentry added to the list
+ * Returns number of dentry added to the list, or -1 on errors
  */
 static int32_t
 quota_add_parents_from_ctx(quota_inode_ctx_t *ctx, struct list_head *list)
@@ -723,15 +727,16 @@ quota_add_parents_from_ctx(quota_inode_ctx_t *ctx, struct list_head *list)
         list_for_each_entry(dentry, &ctx->parents, next)
         {
             ret = quota_add_parent(list, dentry->name, dentry->par);
-
             if (ret == 1)
                 count++;
+            else if (ret == -1)
+                break;
         }
     }
     UNLOCK(&ctx->lock);
 
 out:
-    return count;
+    return (ret == -1) ? -1 : count;
 }
 
 int32_t
@@ -750,10 +755,9 @@ quota_build_ancestry_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
     quota_dentry_t *dentry = NULL;
     quota_dentry_t *tmp = NULL;
     quota_inode_ctx_t *ctx = NULL;
-    struct list_head parents = {
-        0,
-    };
+    struct list_head parents;
     quota_local_t *local = NULL;
+    int ret;
 
     INIT_LIST_HEAD(&parents);
 
@@ -828,7 +832,11 @@ quota_build_ancestry_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
 
     quota_inode_ctx_get(local->loc.inode, this, &ctx, 0);
 
-    quota_add_parents_from_ctx(ctx, &parents);
+    ret = quota_add_parents_from_ctx(ctx, &parents);
+    if (ret == -1) {
+        op_errno = errno;
+        goto err;
+    }
 
     if (list_empty(&parents)) {
         /* we built ancestry for a directory */
@@ -843,7 +851,11 @@ quota_build_ancestry_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
            GF_ASSERT (&entry->list != &entries->list);
         */
 
-        quota_add_parent(&parents, entry->d_name, parent->gfid);
+        ret = quota_add_parent(&parents, entry->d_name, parent->gfid);
+        if (ret == -1) {
+            op_errno = errno;
+            goto err;
+        }
     }
 
     local->ancestry_cbk(&parents, local->loc.inode, 0, 0, local->ancestry_data);
@@ -861,9 +873,11 @@ cleanup:
         parent = NULL;
     }
 
-    list_for_each_entry_safe(dentry, tmp, &parents, next)
-    {
-        __quota_dentry_free(dentry);
+    if (!list_empty(&parents)) {
+        list_for_each_entry_safe(dentry, tmp, &parents, next)
+        {
+            __quota_dentry_free(dentry);
+        }
     }
 
     return 0;
@@ -1839,9 +1853,7 @@ quota_writev(call_frame_t *frame, xlator_t *this, fd_t *fd,
     quota_inode_ctx_t *ctx = NULL;
     quota_dentry_t *dentry = NULL, *tmp = NULL;
     call_stub_t *stub = NULL;
-    struct list_head head = {
-        0,
-    };
+    struct list_head head;
     inode_t *par_inode = NULL;
 
     priv = this->private;
@@ -1881,9 +1893,13 @@ quota_writev(call_frame_t *frame, xlator_t *this, fd_t *fd,
     priv = this->private;
     GF_VALIDATE_OR_GOTO(this->name, priv, unwind);
 
-    size = iov_length(vector, count);
-
     parents = quota_add_parents_from_ctx(ctx, &head);
+    if (parents == -1) {
+        op_errno = errno;
+        goto unwind;
+    }
+
+    size = iov_length(vector, count);
 
     LOCK(&local->lock);
     {
@@ -4805,6 +4821,10 @@ quota_fallocate(call_frame_t *frame, xlator_t *this, fd_t *fd, int32_t mode,
     GF_VALIDATE_OR_GOTO(this->name, priv, unwind);
 
     parents = quota_add_parents_from_ctx(ctx, &head);
+    if (parents == -1) {
+        op_errno = errno;
+        goto unwind;
+    }
 
     /*
      * Note that by using len as the delta we're assuming the range from
diff --git a/xlators/performance/open-behind/src/open-behind.c b/xlators/performance/open-behind/src/open-behind.c
index a4302a776c..268c7176f0 100644
--- a/xlators/performance/open-behind/src/open-behind.c
+++ b/xlators/performance/open-behind/src/open-behind.c
@@ -347,12 +347,14 @@ ob_inode_wake(xlator_t *this, struct list_head *ob_fds)
     ob_fd_t *ob_fd = NULL, *tmp = NULL;
     fd_t *fd = NULL;
 
-    list_for_each_entry_safe(ob_fd, tmp, ob_fds, ob_fds_on_inode)
-    {
-        ob_fd_wake(this, ob_fd->fd, ob_fd);
-        fd = ob_fd->fd;
-        ob_fd_free(ob_fd);
-        fd_unref(fd);
+    if (!list_empty(ob_fds)) {
+        list_for_each_entry_safe(ob_fd, tmp, ob_fds, ob_fds_on_inode)
+        {
+            ob_fd_wake(this, ob_fd->fd, ob_fd);
+            fd = ob_fd->fd;
+            ob_fd_free(ob_fd);
+            fd_unref(fd);
+        }
     }
 }
 
@@ -381,9 +383,7 @@ open_all_pending_fds_and_resume(xlator_t *this, inode_t *inode,
     ob_fd_t *ob_fd = NULL, *tmp = NULL;
     gf_boolean_t was_open_in_progress = _gf_false;
     gf_boolean_t wait_for_open = _gf_false;
-    struct list_head ob_fds = {
-        0,
-    };
+    struct list_head ob_fds;
 
     ob_inode = ob_inode_get(this, inode);
     if (ob_inode == NULL)
diff --git a/xlators/performance/write-behind/src/write-behind.c b/xlators/performance/write-behind/src/write-behind.c
index f90abad38d..98b2f4639d 100644
--- a/xlators/performance/write-behind/src/write-behind.c
+++ b/xlators/performance/write-behind/src/write-behind.c
@@ -1744,15 +1744,9 @@ wb_do_winds(wb_inode_t *wb_inode, list_head_t *tasks)
 void
 wb_process_queue(wb_inode_t *wb_inode)
 {
-    list_head_t tasks = {
-        0,
-    };
-    list_head_t lies = {
-        0,
-    };
-    list_head_t liabilities = {
-        0,
-    };
+    list_head_t tasks;
+    list_head_t lies;
+    list_head_t liabilities;
     int wind_failure = 0;
 
     INIT_LIST_HEAD(&tasks);
@@ -1773,15 +1767,18 @@ wb_process_queue(wb_inode_t *wb_inode)
         }
         UNLOCK(&wb_inode->lock);
 
-        wb_do_unwinds(wb_inode, &lies);
+        if (!list_empty(&lies))
+            wb_do_unwinds(wb_inode, &lies);
 
-        wb_do_winds(wb_inode, &tasks);
+        if (!list_empty(&tasks))
+            wb_do_winds(wb_inode, &tasks);
 
         /* If there is an error in wb_fulfill before winding write
          * requests, we would miss invocation of wb_process_queue
          * from wb_fulfill_cbk. So, retry processing again.
          */
-        wind_failure = wb_fulfill(wb_inode, &liabilities);
+        if (!list_empty(&liabilities))
+            wind_failure = wb_fulfill(wb_inode, &liabilities);
     } while (wind_failure);
 
     return;
diff --git a/xlators/protocol/client/src/client-lk.c b/xlators/protocol/client/src/client-lk.c
index 01613b7ab6..679e1982f4 100644
--- a/xlators/protocol/client/src/client-lk.c
+++ b/xlators/protocol/client/src/client-lk.c
@@ -360,10 +360,12 @@ delete_granted_locks_owner(fd_t *fd, gf_lkowner_t *owner)
 
     pthread_spin_unlock(&conf->fd_lock);
 
-    list_for_each_entry_safe(lock, tmp, &delete_list, list)
-    {
-        list_del_init(&lock->list);
-        destroy_client_lock(lock);
+    if (!list_empty(&delete_list)) {
+        list_for_each_entry_safe(lock, tmp, &delete_list, list)
+        {
+            list_del_init(&lock->list);
+            destroy_client_lock(lock);
+        }
     }
 
     /* FIXME: Need to actually print the locks instead of count */
diff --git a/xlators/storage/posix/src/posix-inode-handle.h b/xlators/storage/posix/src/posix-inode-handle.h
index 2a4b643815..2009421cdb 100644
--- a/xlators/storage/posix/src/posix-inode-handle.h
+++ b/xlators/storage/posix/src/posix-inode-handle.h
@@ -91,8 +91,10 @@
                        (loc)->path);                                           \
             }                                                                  \
             break;                                                             \
+        } /* __ret == -1 && errno == ELOOP */                                  \
+        else {                                                                 \
+            op_ret = -1;                                                       \
         }                                                                      \
-        /* __ret == -1 && errno == ELOOP */                                    \
     } while (0)
 
 #define POSIX_ANCESTRY_PATH (1 << 0)