hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Colin Patrick McCabe (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-3920) libwebdhfs code cleanup: string processing and using strerror consistently to handle all errors
Date Tue, 16 Oct 2012 17:01:04 GMT

    [ https://issues.apache.org/jira/browse/HDFS-3920?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13477160#comment-13477160
] 

Colin Patrick McCabe commented on HDFS-3920:
--------------------------------------------

Thanks, Jing.  This is a big improvement.

{code}
    if (!resp || !resp->body || !resp->body->content) {
        fprintf(stderr,
                "ERROR: the user-provided buffer should not be NULL!\n");
        return EINVAL;
    }
{code}

This message seems a little misleading, since the buffer may not be {{NULL}}; perhaps only
{{resp->body->content}} is {{NULL}}, for example.  Maybe "invalid user-provided buffer"?

In launchWrite, you have this code:
{code}
    ret = initResponseBuffer(&(resp->body));
    if (ret) {
        goto done;
    }
    ...
    struct curl_slist *chunk = NULL;
    ...
done:
    curl_slist_free_all(chunk);
{code}

The problem here is that "chunk" is not initialized until its C99-style declaration.  So if
you take the first goto, what you've got there is an uninitialized variable :(  This sort
of gotcha is why I usually don't use C99-style declarations.  I also checked the man page
for {{curl_slist_free_all}}, and it doesn't specify whether it handles {{NULL}} pointers transparently.
 You better not pass it {{NULL}}, just in case.

{code}
    const char *responseCode1 = "307 TEMPORARY_REDIRECT";
    const char *responseCode2 = "200 OK";
{code}

Should have a macro or {{static const char * const}} for this?

{code}
tOffset hdfsGetDefaultBlockSize(hdfsFS fs)
{
    errno = EOPNOTSUPP;
    return -1;
}
{code}

I think {{EOPNOTSUP}} is #defined to be the same thing as {{ENOTSUP}} on Linux.  I think we
use {{ENOTSUP}} elsewhere, though; should we be consistent and continue to use it here too?
                
> libwebdhfs code cleanup: string processing and using strerror consistently to handle
all errors
> -----------------------------------------------------------------------------------------------
>
>                 Key: HDFS-3920
>                 URL: https://issues.apache.org/jira/browse/HDFS-3920
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>            Reporter: Jing Zhao
>            Assignee: Jing Zhao
>         Attachments: HDFS-3920-001.patch, HDFS-3920-001.patch, HDFS-3920-002.patch, HDFS-3920-003.patch,
HDFS-3920-004.patch, HDFS-3920-005.patch, HDFS-3920-006.patch
>
>
> 1. Clean up code for string processing;
> 2. Using strerror consistently for error handling;
> 3. Use sprintf to replace decToOctal
> 4. other issues requiring fixing.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message