[ https://issues.apache.org/jira/browse/HDFS-7018?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14274827#comment-14274827 ]
Zhanwei Wang commented on HDFS-7018: ------------------------------------ Hi [~cmccabe] bq. Please, let's just use the regular {{strdup}} provided by the system. Then we also don't have to worry about "array delete" versus regular delete either. Yes, but it introduce more trouble. 1) {{strdup}} use {{malloc}} to allocate memory, then we have to worry about "free" versus regular delete. 2) it cannot handle NULL input, that means we have to handle NULL outside and introduce many duplicate code. 3) Instead of throw std::bad_alloc, we have to handle OOM when {{strdup}} returns NULL, and introduce more duplicate code. We expect to handle all OOM in one place. bq. We should be looking at CLASSPATH and searching all those directories for XML files, so that we can be compatible with the existing libhdfs code. Let's add this feature in another JIRA. bq. {{PARAMETER_ASSERT}}: this isn't what people usually mean by an {{assert}}. ... I would prefer not to have this macro at all ... I also think it's confusing to have "return" statements in macros... How about change its name to {{CHECK_INPUT_PARAMETER}}? We have to check the user input parameters in APIs, right? Macro can reduce the duplicated works (set error message, set errno ...). If you still have concern, it is ok to me to remove this macro and extend its content manually like this and provide informative error message. {code} tSize hdfsWrite(hdfsFS fs, hdfsFile file, const void *buffer, tSize length) { if (NULL == fs) { SetErrorMessage("Input parameter \"fs\" is NULL"); errno = EINVAL; return -1; } if (NULL == file) { SetErrorMessage("Input parameter \"file\" is NULL"); errno = EINVAL; return -1; } if (false == file->isInput()) { SetErrorMessage("Input parameter \"file\" is not writable"); errno = EINVAL; return -1; } if (NULL == buffer) { SetErrorMessage("Input parameter \"buffer\" is NULL"); errno = EINVAL; return -1; } if (length <= 0) { SetErrorMessage("Input parameter \"length\" should larger than 0"); errno = EINVAL; return -1; } ... } {code} bq. Since we're adopting the Google C++ style, we will eventually remove the throw statements from other parts of the code I do not think we will eventually remove the throw statements. Just like Google said in Google C++ style. http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Exceptions bq. On their face, the benefits of using exceptions outweigh the costs, especially in new projects. However, for existing code, the introduction of exceptions has implications on all dependent code. The only thing the Google concerns is that the existing code cannot handle exceptions. libhdfs3 is a new project and we also decided not to make the exceptions cross the boundary of the APIs. Existing code can work well with libhdfs3 and do not worry about the exceptions. So we can use exception internally to significantly reduce the complexity of error handling in libhdfs3. bq. I see this repeated a lot in the code. Why can't we use {{CreateStatusFromException}} to figure out exactly what is wrong, and derive the errno and error message from the Status object? We have used {{CreateStatusFromException}} to handle the exceptions in {{C++}} APIs. And in C APIs, we only need to take care one case, that is {{std::bad_alloc}}. Please note that {{CreateStatusFromException}} will throw {{std::bad_alloc}}. As you have commented previously, we also have to add {{catch (...)}} , since it is {{C++}} code. So although it repeated a lot in code, I do not think it can be removed. bq. I'm not familiar with the "get delegation token", "free delegation token" APIs and we need to discuss what they do and if we want them, etc. I think it's best to file a follow-on for that and leave it out for now. You are right. Let's discuss it in another jira to make this jira simple. > Implement C interface for libhdfs3 > ---------------------------------- > > Key: HDFS-7018 > URL: https://issues.apache.org/jira/browse/HDFS-7018 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: hdfs-client > Reporter: Zhanwei Wang > Assignee: Zhanwei Wang > Attachments: HDFS-7018-pnative.002.patch, > HDFS-7018-pnative.003.patch, HDFS-7018.patch > > > Implement C interface for libhdfs3 -- This message was sent by Atlassian JIRA (v6.3.4#6332)