From d2d95bfe886a7fdf9d58fd5c47ec7c0158393afb Mon Sep 17 00:00:00 2001 From: Colin Patrick Mccabe Date: Thu, 28 May 2015 11:23:44 -0700 Subject: [PATCH] HDFS-8407. libhdfs hdfsListDirectory must set errno to 0 on success (Masatake Iwasaki via Colin P. McCabe) --- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 +++ .../src/main/native/libhdfs/expect.h | 18 ++++++++++++++++++ .../hadoop-hdfs/src/main/native/libhdfs/hdfs.c | 1 + .../hadoop-hdfs/src/main/native/libhdfs/hdfs.h | 3 ++- .../native/libhdfs/test/test_libhdfs_ops.c | 9 +++++++++ .../native/libhdfs/test_libhdfs_threaded.c | 17 ++++++++++++++++- 6 files changed, 49 insertions(+), 2 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 47481a7f721..1c9b25ea229 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -813,6 +813,9 @@ Release 2.8.0 - UNRELEASED HDFS-8431. hdfs crypto class not found in Windows. (Anu Engineer via cnauroth) + HDFS-8407. hdfsListDirectory must set errno to 0 on success (Masatake + Iwasaki via Colin P. McCabe) + Release 2.7.1 - UNRELEASED INCOMPATIBLE CHANGES diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/expect.h b/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/expect.h index e64b108e200..49aa2850afa 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/expect.h +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/expect.h @@ -48,6 +48,24 @@ struct hdfsFile_internal; } \ } while (0); +#define EXPECT_NULL_WITH_ERRNO(x, e) \ + do { \ + const void* __my_ret__ = x; \ + int __my_errno__ = errno; \ + if (__my_ret__ != NULL) { \ + fprintf(stderr, "TEST_ERROR: failed on %s:%d (errno: %d): " \ + "got non-NULL value %p from %s\n", \ + __FILE__, __LINE__, __my_errno__, __my_ret__, #x); \ + return -1; \ + } \ + if (__my_errno__ != e) { \ + fprintf(stderr, "TEST_ERROR: failed on %s:%d (errno: %d): " \ + "got expected NULL without expected errno %d from %s\n", \ + __FILE__, __LINE__, __my_errno__, e, #x); \ + return -1; \ + } \ + } while (0); + #define EXPECT_NONNULL(x) \ do { \ const void* __my_ret__ = x; \ diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/hdfs.c b/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/hdfs.c index 504d47e85e4..a3769fcbbc1 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/hdfs.c +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/hdfs.c @@ -3257,6 +3257,7 @@ done: return NULL; } *numEntries = jPathListSize; + errno = 0; return pathList; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/hdfs.h b/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/hdfs.h index 5b7bc1e3867..c1515d7f2ac 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/hdfs.h +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/hdfs.h @@ -671,7 +671,8 @@ extern "C" { * @param path The path of the directory. * @param numEntries Set to the number of files/directories in path. * @return Returns a dynamically-allocated array of hdfsFileInfo - * objects; NULL on error. + * objects; NULL on error or empty directory. + * errno is set to non-zero on error or zero on success. */ LIBHDFS_EXTERNAL hdfsFileInfo *hdfsListDirectory(hdfsFS fs, const char* path, diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/test/test_libhdfs_ops.c b/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/test/test_libhdfs_ops.c index aa9441a0ad8..f564de45684 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/test/test_libhdfs_ops.c +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/test/test_libhdfs_ops.c @@ -306,6 +306,15 @@ int main(int argc, char **argv) { fprintf(stderr, "waah! hdfsGetPathInfo for %s - FAILED!\n", slashTmp); } + fileList = 0; + fileList = hdfsListDirectory(fs, newDirectory, &numEntries); + if (!(fileList == NULL && numEntries == 0 && !errno)) { + fprintf(stderr, "waah! hdfsListDirectory for empty %s - FAILED!\n", newDirectory); + totalResult++; + } else { + fprintf(stderr, "hdfsListDirectory for empty %s - SUCCESS!\n", newDirectory); + } + fileList = 0; if((fileList = hdfsListDirectory(fs, slashTmp, &numEntries)) != NULL) { for(i=0; i < numEntries; ++i) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/test_libhdfs_threaded.c b/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/test_libhdfs_threaded.c index 17ff7a8367d..702430ce751 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/test_libhdfs_threaded.c +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/native/libhdfs/test_libhdfs_threaded.c @@ -142,7 +142,7 @@ static int doTestHdfsOperations(struct tlhThreadInfo *ti, hdfsFS fs, { char tmp[4096]; hdfsFile file; - int ret, expected; + int ret, expected, numEntries; hdfsFileInfo *fileInfo; struct hdfsReadStatistics *readStats = NULL; @@ -153,6 +153,14 @@ static int doTestHdfsOperations(struct tlhThreadInfo *ti, hdfsFS fs, EXPECT_ZERO(doTestGetDefaultBlockSize(fs, paths->prefix)); + /* There should be no entry in the directory. */ + errno = EACCES; // see if errno is set to 0 on success + EXPECT_NULL_WITH_ERRNO(hdfsListDirectory(fs, paths->prefix, &numEntries), 0); + if (numEntries != 0) { + fprintf(stderr, "hdfsListDirectory set numEntries to " + "%d on empty directory.", numEntries); + } + /* There should not be any file to open for reading. */ EXPECT_NULL(hdfsOpenFile(fs, paths->file1, O_RDONLY, 0, 0, 0)); @@ -179,6 +187,13 @@ static int doTestHdfsOperations(struct tlhThreadInfo *ti, hdfsFS fs, EXPECT_ZERO(hdfsHSync(fs, file)); EXPECT_ZERO(hdfsCloseFile(fs, file)); + /* There should be 1 entry in the directory. */ + EXPECT_NONNULL(hdfsListDirectory(fs, paths->prefix, &numEntries)); + if (numEntries != 1) { + fprintf(stderr, "hdfsListDirectory set numEntries to " + "%d on directory containing 1 file.", numEntries); + } + /* Let's re-open the file for reading */ file = hdfsOpenFile(fs, paths->file1, O_RDONLY, 0, 0, 0); EXPECT_NONNULL(file);