libglusterfsclient: Revert and re-do readdir conformance

This commit basically reverts the previous readdir conformance
patch I sent a few days back. That commit had a completely retarded
and broken way of maintaining per-directory dirent.

It was broken for two reasons:
1. Creating a wrapper structure around the directory's fd_t
only for storing a struct dirent is not clean enough. This commit
takes a better approach by storing the dirent in fd_t context.
This dirent is valid only if the fd_t refers to a directory.

2. That commit was made and tested under the assumption (..stupidity
is a better word..) that only opendir call is used for opening a
directory. That is not correct. Directories are also opened using the
open syscall. The point is, glusterfs_open returns an fd_t and so did
glusterfs_opendir. The previous patch actually changed opendir to
return a new wrapper structure. That is fine, if we go by the POSIX
definition of open and opendir because, they're both supposed to
return different types, an int and a DIR*. However, in
libglusterfsclient, all other code assumes that directory handles
corresponding to DIR* and file descriptors corresponding to int types
are the same type, resulting in use of the same locking and fd context
addition/extraction code. So a directory opened using opendir returned
a wrapper structure which went down into the libglusterfsclient stack
where some function called a lock on the handle assuming it was an
fd_t, since it is not and dereferencing of the supposed fd->inode->lock
results in a seg fault.

Obviously, this didnt show up till unfs3 used open() to open a
directory and not opendir.

Signed-off-by: Anand V. Avati <avati@amp.gluster.com>

2 files changed, 29 insertions, 61 deletions

diff --git a/libglusterfsclient/src/libglusterfsclient-internals.h b/libglusterfsclient/src/libglusterfsclient-internals.h
index 4cdde26e7..40ad6f692 100755
--- a/libglusterfsclient/src/libglusterfsclient-internals.h
+++ b/libglusterfsclient/src/libglusterfsclient-internals.h
@@ -83,6 +83,11 @@ typedef struct {
         pthread_mutex_t lock;
         off_t offset;
         libglusterfs_client_ctx_t *ctx;
+        /* `man readdir` says readdir is non-re-entrant
+         * only if two readdirs are racing on the same
+         * handle.
+         */
+	struct dirent dirp;
 } libglusterfs_client_fd_ctx_t;
 
 typedef struct libglusterfs_client_async_local {
@@ -231,16 +236,4 @@ struct vmp_entry {
 };
 
 
-/* Internal directory handle inited in opendir.
- * This is needed in order to store a per-handle
- * dirent structure.
- */
-struct libgf_dir_handle {
-        fd_t *dirfd;
-        struct dirent dirp;
-};
-
-
-
-
 #endif
diff --git a/libglusterfsclient/src/libglusterfsclient.c b/libglusterfsclient/src/libglusterfsclient.c
index f16dcb4d1..0cfc23e9e 100755
--- a/libglusterfsclient/src/libglusterfsclient.c
+++ b/libglusterfsclient/src/libglusterfsclient.c
@@ -2120,7 +2120,6 @@ glusterfs_glh_open (glusterfs_handle_t handle, const char *path, int flags,...)
         mode_t mode = 0;
         va_list ap;
         char *pathres = NULL;
-        glusterfs_file_t *fh = NULL;
 
         GF_VALIDATE_OR_GOTO (LIBGF_XL_NAME, ctx, out);
         GF_VALIDATE_ABSOLUTE_PATH_OR_GOTO (LIBGF_XL_NAME, path, out);
@@ -2191,22 +2190,13 @@ glusterfs_glh_open (glusterfs_handle_t handle, const char *path, int flags,...)
                 va_end (ap);
                 op_ret = libgf_client_creat (ctx, &loc, fd, flags, mode);
         } else {
-                if (S_ISDIR (loc.inode->st_mode)) {
-                        /* This could be optimized a bit by finding
-                         * the common functionality between
-                         * glusterfs_glh_open and
-                         * glusterfs_glh_opendir.
-                         */
-                        fh = glusterfs_glh_opendir (ctx, path);
-                        goto out;
-                } else
+                if (S_ISDIR (loc.inode->st_mode))
+                        op_ret = libgf_client_opendir (ctx, &loc, fd);
+                else
                         op_ret = libgf_client_open (ctx, &loc, fd, flags);
         }
 
 op_over:
-        /* For a directory all this is done inside
-         * glusterfs_glh_opendir.
-         */
         if (op_ret == -1) {
                 fd_unref (fd);
                 fd = NULL;
@@ -2229,12 +2219,6 @@ op_over:
                 }
         }
 
-        /* This assignment will happen if the pathname to be opened is
-         * a file, otherwise, if it is a directory, we'll jump to out:
-         * right after the call to glusterfs_glh_opendir above.
-         */
-        fh = (glusterfs_file_t)fd;
-
 out:
         libgf_client_loc_wipe (&loc);
 
@@ -2245,7 +2229,7 @@ out:
         if (pathres)
                 FREE (pathres);
 
-        return fh;
+        return fd;
 }
 
 glusterfs_file_t
@@ -3375,9 +3359,8 @@ glusterfs_readdir (glusterfs_dir_t dirfd)
         off_t offset = 0;
         libglusterfs_client_fd_ctx_t *fd_ctx = NULL;
         struct dirent *dirp = NULL;
-        struct libgf_dir_handle *dir = (struct libgf_dir_handle *)dirfd;
 
-        fd_ctx = libgf_get_fd_ctx (dir->dirfd);
+        fd_ctx = libgf_get_fd_ctx (dirfd);
         if (!fd_ctx) {
                 errno = EBADF;
 		goto out;
@@ -3387,12 +3370,12 @@ glusterfs_readdir (glusterfs_dir_t dirfd)
         {
                 ctx = fd_ctx->ctx;
                 offset = fd_ctx->offset;
+                dirp = &fd_ctx->dirp;
         }
         pthread_mutex_unlock (&fd_ctx->lock);
 
-        dirp = &dir->dirp;
         memset (dirp, 0, sizeof (struct dirent));
-        op_ret = libgf_client_readdir (ctx, dir->dirfd, dirp,
+        op_ret = libgf_client_readdir (ctx, (fd_t *)dirfd, dirp,
                                        LIBGF_READDIR_BLOCK, &offset, 1);
 
         if (op_ret <= 0) {
@@ -4611,7 +4594,6 @@ glusterfs_glh_opendir (glusterfs_handle_t handle, const char *path)
         loc_t                           loc = {0, };
         fd_t                            *dirfd = NULL;
         char                            *name = NULL;
-        struct libgf_dir_handle         *dir = NULL;
 
         GF_VALIDATE_OR_GOTO (LIBGF_XL_NAME, ctx, out);
         GF_VALIDATE_ABSOLUTE_PATH_OR_GOTO (LIBGF_XL_NAME, path, out);
@@ -4638,40 +4620,34 @@ glusterfs_glh_opendir (glusterfs_handle_t handle, const char *path)
                 goto out;
         }
 
-        dir = CALLOC (1, sizeof (struct libgf_dir_handle));
-        if (!dir) {
-                op_ret = -1;
-                goto out;
-        }
-
-        dir->dirfd = fd_create (loc.inode, 0);
-        op_ret = libgf_client_opendir (ctx, &loc, dir->dirfd);
-
-        if (op_ret == -1)
-                goto out;
+        dirfd = fd_create (loc.inode, 0);
+        op_ret = libgf_client_opendir (ctx, &loc, dirfd);
 
-        if (libgf_get_fd_ctx (dir->dirfd))
+        if (op_ret == -1) {
+                fd_unref (dirfd);
+                dirfd = NULL;
                 goto out;
+        }
 
-        if (!(libgf_alloc_fd_ctx (ctx, dir->dirfd))) {
-                op_ret = -1;
-                errno = EINVAL;
-                goto out;
+        if (!libgf_get_fd_ctx (dirfd)) {
+                if (!(libgf_alloc_fd_ctx (ctx, dirfd))) {
+                        op_ret = -1;
+                        errno = EINVAL;
+                        goto out;
+                }
         }
 
-        op_ret = 0;
 out:
         if (name)
                 FREE (name);
 
         if (op_ret == -1) {
-                FREE (dir);
                 fd_unref (dirfd);
-                dir = NULL;
+                dirfd = NULL;
         }
 
         libgf_client_loc_wipe (&loc);
-        return (glusterfs_dir_t)dir;
+        return dirfd;
 }
 
 glusterfs_dir_t
@@ -4704,7 +4680,7 @@ glusterfs_closedir (glusterfs_dir_t dirfd)
         libglusterfs_client_fd_ctx_t    *fdctx = NULL;
 
         GF_VALIDATE_OR_GOTO (LIBGF_XL_NAME, dirfd, out);
-        fdctx = libgf_get_fd_ctx (((struct libgf_dir_handle *)dirfd)->dirfd);
+        fdctx = libgf_get_fd_ctx (dirfd);
 
         if (fdctx == NULL) {
                 errno = EBADF;
@@ -4712,9 +4688,8 @@ glusterfs_closedir (glusterfs_dir_t dirfd)
                 goto out;
         }
 
-        op_ret = libgf_client_flush (fdctx->ctx,
-                                    ((struct libgf_dir_handle *)dirfd)->dirfd);
-        fd_unref (((struct libgf_dir_handle *)dirfd)->dirfd);
+        op_ret = libgf_client_flush (fdctx->ctx, (fd_t *)dirfd);
+        fd_unref ((fd_t *)dirfd);
 
 out:
         return op_ret;