[kudu-CR] Fix encoding-test on OS X

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Fix encoding-test on OS X
..


Patch Set 2: -Verified

Build Started http://104.196.14.100/job/kudu-gerrit/2426/

-- 
To view, visit http://gerrit.cloudera.org:8080/3640
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie221906ee3960fa158843c2ae2dd7a531e74c5c4
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No

[kudu-CR] Fix encoding-test on OS X

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Fix encoding-test on OS X
..


Patch Set 2: -Verified

Build Started http://104.196.14.100/job/kudu-gerrit/2413/

-- 
To view, visit http://gerrit.cloudera.org:8080/3640
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie221906ee3960fa158843c2ae2dd7a531e74c5c4
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No

[kudu-CR] Fix encoding-test on OS X

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

Change subject: Fix encoding-test on OS X
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3640/1/src/kudu/cfile/encoding-test.cc
File src/kudu/cfile/encoding-test.cc:

Line 80:   string *val = new string(StringPrintf(fmt_str, i));
> This is leaking the string since it's never deallocated.  I think the fix h
Yeah my bad...that was pretty obvious. Thanks for the quick comment.


-- 
To view, visit http://gerrit.cloudera.org:8080/3640
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie221906ee3960fa158843c2ae2dd7a531e74c5c4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] Fix encoding-test on OS X

[Will Berkeley (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/3640

to look at the new patch set (#2).

Change subject: Fix encoding-test on OS X
..

Fix encoding-test on OS X

A recent change caused encoding-test to fail on OS X. This reverts
the part of the change that causes the test to fail (and nothing
else).

I'm not sure why it fails on OS X and not in, e.g., Jenkins builds.

Change-Id: Ie221906ee3960fa158843c2ae2dd7a531e74c5c4
---
M src/kudu/cfile/encoding-test.cc
1 file changed, 5 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/3640/2
-- 
To view, visit http://gerrit.cloudera.org:8080/3640
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie221906ee3960fa158843c2ae2dd7a531e74c5c4
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Fix encoding-test on OS X

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Fix encoding-test on OS X
..


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/2396/

-- 
To view, visit http://gerrit.cloudera.org:8080/3640
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie221906ee3960fa158843c2ae2dd7a531e74c5c4
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Fix encoding-test on OS X

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: Fix encoding-test on OS X
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3640/1/src/kudu/cfile/encoding-test.cc
File src/kudu/cfile/encoding-test.cc:

Line 80:   string *val = new string(StringPrintf(fmt_str, i));
This is leaking the string since it's never deallocated.  I think the fix here 
is to have change to_insert to be a 'vector' like this:


diff --git a/src/kudu/cfile/encoding-test.cc b/src/kudu/cfile/encoding-test.cc
index 0e6a6d9..40d61a2 100644
--- a/src/kudu/cfile/encoding-test.cc
+++ b/src/kudu/cfile/encoding-test.cc
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 

@@ -39,6 +40,7 @@
 #include "kudu/util/test_macros.h"
 #include "kudu/util/stopwatch.h"

+using std::unique_ptr;
 using std::vector;

 namespace kudu { namespace cfile {
@@ -73,15 +75,14 @@ class TestEncoding : public ::testing::Test {
   static Slice CreateBinaryBlock(BuilderType *sbb,
  int num_items,
  const char *fmt_str) {
-vector to_insert;
+vector to_insert;
 std::vector slices;

 for (uint i = 0; i < num_items; i++) {
-  to_insert.emplace_back(StringPrintf(fmt_str, i));
-  slices.push_back(Slice(to_insert.back()));
+  to_insert.emplace_back(new string(StringPrintf(fmt_str, i)));
+  slices.push_back(Slice(to_insert.back()->data()));
 }

-
 int rem = slices.size();
 Slice *ptr = [0];
 while (rem > 0) {


-- 
To view, visit http://gerrit.cloudera.org:8080/3640
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie221906ee3960fa158843c2ae2dd7a531e74c5c4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Fix encoding-test on OS X

[Will Berkeley (Code Review)]

Will Berkeley has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/3640

Change subject: Fix encoding-test on OS X
..

Fix encoding-test on OS X

A recent change caused encoding-test to fail on OS X. This reverts
the part of the change that causes the test to fail (and nothing
else).

I'm not sure why it fails on OS X and not in, e.g., Jenkins builds.

Change-Id: Ie221906ee3960fa158843c2ae2dd7a531e74c5c4
---
M src/kudu/cfile/encoding-test.cc
1 file changed, 3 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/3640/1
-- 
To view, visit http://gerrit.cloudera.org:8080/3640
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie221906ee3960fa158843c2ae2dd7a531e74c5c4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 

[kudu-CR] Fix encoding-test on OS X

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Fix encoding-test on OS X
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/2394/

-- 
To view, visit http://gerrit.cloudera.org:8080/3640
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie221906ee3960fa158843c2ae2dd7a531e74c5c4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No