[
https://issues.apache.org/jira/browse/HDFS-9628?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15090130#comment-15090130
]
James Clampffer commented on HDFS-9628:
---------------------------------------
A couple comments, otherwise it looks good to me.
-In hdfsConfStrFree you don't need a null check before calling free; free does
that anyway.
-Valgrind failed for me, but it looks like a simple fix:
{code}
==23607== Command: ./hdfs_builder_test
==23607==
Running main() from gmock_main.cc
[==========] Running 3 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 3 tests from HdfsBuilderTest
[ RUN ] HdfsBuilderTest.TestStubBuilder
[ OK ] HdfsBuilderTest.TestStubBuilder (35 ms)
[ RUN ] HdfsBuilderTest.TestRead
==23607== Warning: invalid file descriptor -1 in syscall close()
==23607== Warning: invalid file descriptor -1 in syscall close()
[ OK ] HdfsBuilderTest.TestRead (48 ms)
[ RUN ] HdfsBuilderTest.TestSet
[ OK ] HdfsBuilderTest.TestSet (10 ms)
[----------] 3 tests from HdfsBuilderTest (102 ms total)
[----------] Global test environment tear-down
[==========] 3 tests from 1 test case ran. (128 ms total)
[ PASSED ] 3 tests.
{code}
-Unlikely to happen but
{code}
TempFile(const std::string & fn) : filename(fn), tempFileHandle(-1) {
strncpy(fn_buffer, fn.c_str(), sizeof(fn_buffer));
}
{code}
If the length of fn is greater than or equal to sizeof(fn_buffer) strncpy isn't
going to null terminate the string being copied.
> libhdfs++: Implement builder apis from C bindings
> -------------------------------------------------
>
> Key: HDFS-9628
> URL: https://issues.apache.org/jira/browse/HDFS-9628
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
> Reporter: Bob Hansen
> Assignee: Bob Hansen
> Attachments: HDFS-9628.HDFS-8707.000.patch,
> HDFS-9628.HDFS-8707.001.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
