Details
-
Improvement
-
Status: Open
-
Major
-
Resolution: Unresolved
-
None
-
None
-
None
Description
The usage of errno in libhdfs has gone through several changes in the past: HDFS-3675, HDFS-4997, HDFS-3579, HDFS-8407, etc.
As a result of these changes, some libhdfs functions set errno to 0 on success (hadoopReadZero, hdfsListDirectory), while several set errno to a meaningful value only on error.
libhdfs++ on the other hand sets errno to 0 for all (successful) libhdfs++ operations. See this comment in HDFS-10511 for why that was done.
The inconsistent behavior between libhdfs and libhdfs++ causes issues for tests such as test_hdfs_ext_hdfspp_test_shim_static which uses a shim layer (tests/hdfs_shim.c) that delegates to both hdfs.c and hdfs.cc for various operations (e.g. opening / closing files uses both APIs, hdfsWrite delegates to libhdfs since libhdfs++ does not support writes yet). The tests expect errno to be set to 0 after successful operations against the shim layer. Since libhdfs is not guaranteed to set errno to 0 on success, tests can start failing.
One example of the inconsistency causing issues is HDFS-14111, the patch for HDFS-14111 happens to change the errno from 0 to 2 for hdfsCloseFile. However, from libhdfs's perspective this seems to be by design. Quoting from the errno C docs:
The value in errno is significant only when the return value of the call indicated an error (i.e., -1 from most system calls; -1 or NULL from most library functions); a function that succeeds is allowed to change errno.
I was not able to pin down why the patch for HDFS-14111 changed the errno value, but I isolated the change to the FileSystem#close call. Most likely, some C function invoked as a result of calling #close failed and changed the errno value, but the #close was still able to succeed (this is most likely expected behavior, see this comment in HDFS-8407 for further validation).
Going forward we could (1) set errno to 0 for all successful libhdfs functions, which would make the libhdfs behavior consistent with the libhdfs++ behavior, or (2) we could live with the discrepancy, which would require modifying from the libhdfs++ shim tests (which assert that errno is 0 after certain operations) and just document the difference in behavior.