[
https://issues.apache.org/jira/browse/HDFS-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14551225#comment-14551225
]
Colin Patrick McCabe commented on HDFS-8407:
--------------------------------------------
Thanks for looking at this, [~iwasakims]. One problem I see here is that we
can't easily distinguish between NULL being returned because there is an error,
and NULL being returned because there was an empty directory. In both cases,
{{errno}} may be set.
Here is the code in {{hdfs.c}}:
{code}
...
ret = 0;
done:
destroyLocalReference(env, jPath);
destroyLocalReference(env, jPathList);
if (ret) {
hdfsFreeFileInfo(pathList, jPathListSize);
errno = ret;
return NULL;
}
*numEntries = jPathListSize;
return pathList;
{code}
Note that {{errno}} is only set when there is an error. When there is no
error, {{errno}} may have any value. Remember that lots of C library functions
set {{errno}}. Just because some random C library function failed, doesn't
mean we want to return failure.
I think the current patch is a good start, but we should also explicitly set
{{errno}} = 0 on success. We should also document in the header file that
{{errno}} will be set to 0 if there is no error.
> libhdfs hdfsListDirectory() API has different behavior than documentation
> -------------------------------------------------------------------------
>
> Key: HDFS-8407
> URL: https://issues.apache.org/jira/browse/HDFS-8407
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: HDFS
> Reporter: Juan Yu
> Assignee: Masatake Iwasaki
> Attachments: HDFS-8407.001.patch, HDFS-8407.002.patch
>
>
> The documentation says it returns NULL on error, but it could also return
> NULL when the directory is empty.
> /**
> * hdfsListDirectory - Get list of files/directories for a given
> * directory-path. hdfsFreeFileInfo should be called to deallocate
> memory.
> * @param fs The configured filesystem handle.
> * @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.
> */
> {code}
> hdfsFileInfo *pathList = NULL;
> ...
> //Figure out the number of entries in that directory
> jPathListSize = (*env)->GetArrayLength(env, jPathList);
> if (jPathListSize == 0) {
> ret = 0;
> goto done;
> }
> ...
> if (ret) {
> hdfsFreeFileInfo(pathList, jPathListSize);
> errno = ret;
> return NULL;
> }
> *numEntries = jPathListSize;
> return pathList;
> {code}
> Either change the implementation to match the doc, or fix the doc to match
> the implementation.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
