hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Anu Engineer (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-12340) Ozone: C/C++ implementation of ozone client using curl
Date Mon, 28 Aug 2017 19:59:00 GMT

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

Anu Engineer commented on HDFS-12340:
-------------------------------------

[~shashikant] Thank you for sharing the work in progress. Some minor comments

# nit: add comments for functions and rename the file extension to .c instead of .C
#  handleCreate* functions -- assert that info is not null.
# In function main -- vol and bucket max size is 64 bytes. key size max is 1024 -- 
# always use curly ? 
 {{if (rc != 0)
    return -1;}} 
# OzoneClient.c {{initOzoneClient}} -- init the vars, As a habit :
{code} 
char user[64] = {0};
 char version[64]= {0};
 char url[256]= {0};
 CURLcode res = 0;
{code}
# printf("invalid or no user name provided\n"); --> printf to stderror please.
# Missing goto exit
{code}
if (!isValidStr(userName))
 {
    printf("invalid or no user name provided\n");
    rc = -1;
    >> missing goto exit <<
 }    
{code}
# Check return values
{code}
ozoneInfo *info = (ozoneInfo *) malloc(sizeof(struct ozoneInfo));
info->url = (char *) malloc(strlen(url) +1);
info->version = (char *) malloc(strlen(ozoneVersion) +1);
check if the malloc failed, and bail if needed.
{code}
# Also memset or use calloc here, so these structs are all clean.
# use something which checks the array bounds while doing the copy in getUrlHead or check
them yourself, please.
# There is a memory leak here.
{code}
else
 {
   free(info);
   return NULL;
 }
{code}
You should free all the internally allocated variables before calling this free.
# resetOzoneInfo -- check if info is valid before accessing this variable.
# Init all the variables, please.
# replace strcat/strcpy with safer functions.
# Is there a memory leak in this function-- should we call curl_slist_free_all before adding
to this list ? 
# curl_slist_append - check the return is not null -- at all places.
# Set info to NULL after destroy is called?
# check *chunk is valid in addDateHeader
# executeCurlPostOperation -- check info is valid before dereferencing ? 
# if (rc == -1)
      goto exit; -- put curly brackets? 
# Missing "goto exit" in createVolume, createBucket, putKey after the rc = -1;


> Ozone: C/C++ implementation of ozone client using curl
> ------------------------------------------------------
>
>                 Key: HDFS-12340
>                 URL: https://issues.apache.org/jira/browse/HDFS-12340
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: ozone
>    Affects Versions: HDFS-7240
>            Reporter: Shashikant Banerjee
>            Assignee: Shashikant Banerjee
>             Fix For: HDFS-7240
>
>         Attachments: main.C, ozoneClient.C, ozoneClient.h
>
>
> This Jira is introduced for implementation of ozone client in C/C++ using curl library.
> All these calls will make use of HTTP protocol and would require libcurl. The libcurl
API are referenced from here:
> https://curl.haxx.se/libcurl/
> Additional details would be posted along with the patches.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-help@hadoop.apache.org


Mime
View raw message