[ https://issues.apache.org/jira/browse/HDFS-8766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14971680#comment-14971680 ]
Haohui Mai commented on HDFS-8766: ---------------------------------- {code} +add_library(bindings hdfs.cc) +add_dependencies(bindings fs rpc reader proto common fs rpc reader proto common) {code} It makes more sense to put all the files further down to {{bindings/c}} as there will be JNI bindings in {{bindings/jni}}. {code} +#include "stdint.h" +#include "errno.h" {code} Should be {{cstdint}} and {{cerrno}} {code} +#include "stdint.h" +#include "errno.h" +#include "../../../libhdfs/include/hdfs/hdfs.h" + +#include "libhdfspp/hdfs.h" +#include "libhdfspp/status.h" +#include "fs/filesystem.h" +#include "common/hdfs_public_api.h" + +#include <string> //to_string, string +#include <future> //promise, future +#include <memory> //make_shared +#include <thread> +#include <vector> +#include <set> + {code} The comments for the include files are unnecessary. The order of include files should be (1) local headers, (2) headers that are specific to the projects and (3) C++ headers. It avoid unnecessary loading. {code} +#include "../../../libhdfs/include/hdfs/hdfs.h" {code} Headers that are external to the projects are supposed to be included via bracket. It makes sense to add {{libhdfs/include}} in the {{include_directories}} in {{CMakeLists.txt}} so that you can use {{#include <hdfs/hdfs.h>}} here. {code} +int HdfsInternal::AddWorkerThread() { ... {code} Let's separate the implementation of {{HdfsInternal}} and the C bindings into different files to make the tasks HDFS-9144 easier. {code} +//--------------------------------------------------------- +// Begin public C API implementations +//--------------------------------------------------------- {code} It makes sense to use the {{/** ... **/}} types of comments to be consistent with the coding styles (which is used by libhdfspp and the Java implementation). I'll comment on implementation details separately. > Implement a libhdfs(3) compatible API > ------------------------------------- > > Key: HDFS-8766 > URL: https://issues.apache.org/jira/browse/HDFS-8766 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client > Reporter: James Clampffer > Assignee: James Clampffer > Attachments: HDFS-8766.HDFS-8707.000.patch, > HDFS-8766.HDFS-8707.001.patch, HDFS-8766.HDFS-8707.002.patch, > HDFS-8766.HDFS-8707.003.patch, HDFS-8766.HDFS-8707.004.patch, > HDFS-8766.HDFS-8707.005.patch, HDFS-8766.HDFS-8707.006.patch, > HDFS-8766.HDFS-8707.007.patch, HDFS-8766.HDFS-8707.008.patch, > HDFS-8766.HDFS-8707.009.patch > > > Add a synchronous API that is compatible with the hdfs.h header used in > libhdfs and libhdfs3. This will make it possible for projects using > libhdfs/libhdfs3 to relink against libhdfspp with minimal changes. > This also provides a pure C interface that can be linked against projects > that aren't built in C++11 mode for various reasons but use the same > compiler. It also allows many other programming languages to access > libhdfspp through builtin FFI interfaces. > The libhdfs API is very similar to the posix file API which makes it easier > for programs built using posix filesystem calls to be modified to access HDFS. -- This message was sent by Atlassian JIRA (v6.3.4#6332)