Jim Apple has posted comments on this change. Change subject: IMPALA-2107: Add Base64 encoder/decoder ......................................................................
Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/2633/2/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: Line 112: * > spacing Done http://gerrit.cloudera.org:8080/#/c/2633/2/be/src/exprs/string-functions.cc File be/src/exprs/string-functions.cc: Line 804: if (UNLIKELY(outmax > static_cast<unsigned>(std::numeric_limits<int>::max()))) { > does this catch all possible overflows? what if str.len == max or max-1? i Good catch. I cast str.len to unsigned, so overflow shouldn't be possible any more. Line 814: outmax > maybe use result.len here and line below. but feel free to ignore if you fi Yes, I prefer out_max here, given the way the method's prototype is declared (with outmax and outlen) Line 840: outmax > out_max Done Line 849: outlen > out_len Done -- To view, visit http://gerrit.cloudera.org:8080/2633 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I911451c5d68e8ae9d352abfcf4d5ff36484f0bf3 Gerrit-PatchSet: 2 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Jim Apple <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-HasComments: Yes
