You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org> on 2017/04/18 18:32:44 UTC
[kudu-CR] Slice::ToDebugString() - Add option to not ascii'ize
David Ribeiro Alves has uploaded a new change for review.
http://gerrit.cloudera.org:8080/6671
Change subject: Slice::ToDebugString() - Add option to not ascii'ize
......................................................................
Slice::ToDebugString() - Add option to not ascii'ize
This method prints bytes individually and always tries to find
a graphical representation of the byte, before outputting the raw
value. This is great when the slices are written messages but
not so good when slices point to raw data, in which case some
bytes will, by chance, correspond to well known ascii chars and
some won't.
To avoid this I've added a new argument, true by default, that when
set to false prevents the transformation and always prints the raw
bytes.
Change-Id: I15ed6f5e18fa20b3dca602c0a215980b97975d05
---
M src/kudu/util/slice-test.cc
M src/kudu/util/slice.cc
M src/kudu/util/slice.h
3 files changed, 9 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/6671/1
--
To view, visit http://gerrit.cloudera.org:8080/6671
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I15ed6f5e18fa20b3dca602c0a215980b97975d05
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
[kudu-CR] Slice::ToDebugString() - Add option to not ascii'ize
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: Slice::ToDebugString() - Add option to not ascii'ize
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6671/1/src/kudu/util/slice.h
File src/kudu/util/slice.h:
Line 180: std::string ToDebugString(size_t max_len = 0, bool try_print_ascii = true) const;
The Slice class is part of the client API, and this change breaks ABI (but not API) compatibility for ToDebugString().
From https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B: "[You cannot... extend] a function with another parameter, even if this parameter has a default argument."
Avoiding this ascii-ization will have to be done in a new method.
--
To view, visit http://gerrit.cloudera.org:8080/6671
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I15ed6f5e18fa20b3dca602c0a215980b97975d05
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Slice::ToDebugString() - Add option to not ascii'ize
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: Slice::ToDebugString() - Add option to not ascii'ize
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6671/1//COMMIT_MSG
Commit Message:
Line 7: Slice::ToDebugString() - Add option to not ascii'ize
instead, would it be better to just use HexDump(slice) for this use case? Or maybe add a new method? I'm not too keen on adding optional parameters to such a commonly-used function.
This is also part of our public ABI so i'm not sure if we can modify the signature without breaking binary compat.
--
To view, visit http://gerrit.cloudera.org:8080/6671
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I15ed6f5e18fa20b3dca602c0a215980b97975d05
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Slice::ToDebugString() - Add option to not ascii'ize
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has abandoned this change.
Change subject: Slice::ToDebugString() - Add option to not ascii'ize
......................................................................
Abandoned
HexDump seems to be the right way to do this
--
To view, visit http://gerrit.cloudera.org:8080/6671
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: abandon
Gerrit-Change-Id: I15ed6f5e18fa20b3dca602c0a215980b97975d05
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>