hadoop-mapreduce-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sean Zhong (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (MAPREDUCE-6005) native-task: fix some valgrind errors
Date Thu, 31 Jul 2014 04:05:39 GMT

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

Sean Zhong commented on MAPREDUCE-6005:
---------------------------------------

1. About toHex
{quote}
-string StringUtil::ToString(const void * v, uint32_t len) {
+static char ToHex(uint8_t v) {
+  return v < 10 ? (v + '0') : (v - 10 + 'a');
+}
{quote}

It is not safe to not doing range check for a public function, besides the correct implementation
which convert binary to string should use base64 encoding. Since StringUtil::ToString(const
void * v, uint32_t len)  is only used for md5 conversion, 
{quote}
  case MD5HashType:
    dest.append(StringUtil::ToString(data, length));
{quote}

I believe we can rename StringUtil::ToString(const void * v, uint32_t len) to StringUtil::md5BinaryToString(const
void * v, uint32_t len), and also make ToHex(uint8_t v) private or inlined to md5BinaryToString.

2. memmov replace memcpy is good, thanks

3. About 
{quote}
       } else { // no more, pop heap
+        delete _heap[0];
{quote}
There is another leak at Merge
{quote}
        MergeEntryPtr * base = &(_heap[0]);
        popHeap(base, base + cur_heap_size, _comparator);
        _heap.pop_back();
{quote}
And, I suggest we can add a comments in source about why we delete _heap[0]


Others looks good, +1

> native-task: fix some valgrind errors 
> --------------------------------------
>
>                 Key: MAPREDUCE-6005
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-6005
>             Project: Hadoop Map/Reduce
>          Issue Type: Sub-task
>          Components: task
>            Reporter: Binglin Chang
>            Assignee: Binglin Chang
>         Attachments: MAPREDUCE-6005.v1.patch, MAPREDUCE-6005.v2.patch
>
>
> Running test with valgrind shows there are some bugs, this jira try to fix them.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Mime
View raw message