Mailing-List: contact mapreduce-issues-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: mapreduce-issues@hadoop.apache.org
Date: Thu, 31 Jul 2014 04:05:39 +0000 (UTC)
From: "Sean Zhong (JIRA)" <jira@apache.org>
To: mapreduce-issues@hadoop.apache.org
Message-ID: <JIRA.12729652.1406260381872.83516.1406779539606@arcas>
In-Reply-To: <JIRA.12729652.1406260381872@arcas>
References: <JIRA.12729652.1406260381872@arcas>
Subject: [jira] [Commented] (MAPREDUCE-6005) native-task: fix some valgrind
 errors
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ 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)