From aeda0a31977e8da295b913e6a306ff01ccf0ce0a Mon Sep 17 00:00:00 2001 From: Shehjar Tikoo Date: Mon, 11 May 2009 18:22:29 +0530 Subject: 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 --- libglusterfsclient/src/libglusterfsclient.c | 73 ++++++++++------------------- 1 file changed, 24 insertions(+), 49 deletions(-) (limited to 'libglusterfsclient/src/libglusterfsclient.c') 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; -- cgit