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