[
https://issues.apache.org/jira/browse/HDFS-9627?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15089463#comment-15089463
]
James Clampffer commented on HDFS-9627:
---------------------------------------
Thanks [~bobhansen] for resubmitting the patch for CI and feedback.
Regarding your comments:
"Should libhdfspp.h live in bindings/c?"
I moved this around a couple times while I was working on it. The reasons for
putting it in include/libhdfspp were both to make packaging easier as well as
try and keep all libhdfspp specific includes in one place. I don't have a
problem with moving it if you'd prefer that.
"Tests should probably be using EXPECT_EQ(expected value, test value) rather
than ASSERT(ex == test)"
Good point, I can change that.
"Additional minor comment: for consistency, we should mark the methods as
LIBHDFS_EXTERNAL, which means including hdfs.h, which means we need to
disambiguate the C hdfs.h from the libdhfdspp hdfs.h (which declares FileSystem
and FileHandle). Perhaps renaming this patch's hdfspp.h to hdfs_ext.h and
renaming libhdfs/include/libhdfspp/hdfs.h to hdfspp.h is a good set of names?"
Good point, I think having two header files with the same name in the same
project is asking for trouble. I've already had some situations writing toy
applications on top of libhdfs++ where that got annoying. I think moving
hdfspp.h->hdfs_extentions and (the libhdfs++) hdfs.h->hdfspp.h would be a good
route. I'll change that and add LIBHDFS_EXTERNAL.
"Additional minor comment: the new method should be declared in the "extern C"
namespace."
Good catch. If more functions start getting added to the extensions header it
may be worth building a test in C99 mode and linking it against the functions
in the header as a way of enforcing this.
> libhdfs++: Add a mechanism to retrieve human readable error messages through
> the C API
> --------------------------------------------------------------------------------------
>
> Key: HDFS-9627
> URL: https://issues.apache.org/jira/browse/HDFS-9627
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
> Reporter: James Clampffer
> Assignee: James Clampffer
> Attachments: HDFS-9627.HDFS-8707.000.patch,
> HDFS-9627.HDFS-8707.000.patch
>
>
> Libhdfs doesn't have this but libhdfs3 has a "hdfsGetLastErrorString"
> function. The C API needs to be able to pass out error messages that are
> more specific than what errno can provide.
> This functionality should be exposed via a new public header in order to keep
> hdfs.h consistent with the libhdfs header.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
