[ 
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)

Reply via email to