[Impala-ASF-CR] IMPALA-5031: remove undefined behavior: call to strncmp with nullptr

2017-05-06 Thread Jim Apple (Code Review) 
Jim Apple has abandoned this change.

Change subject: IMPALA-5031: remove undefined behavior: call to strncmp with 
nullptr
..


Abandoned

Done in another commit

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Id563e81720a0a4847664fa2828ecfdcad870da5b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Jim Apple

[Impala-ASF-CR] IMPALA-5031: remove undefined behavior: call to strncmp with nullptr

2017-05-02 Thread Dan Hecht (Code Review) 
Dan Hecht has posted comments on this change.

Change subject: IMPALA-5031: remove undefined behavior: call to strncmp with 
nullptr
..


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6767/1/be/src/runtime/string-value.inline.h
File be/src/runtime/string-value.inline.h:

Line 59:   const int result = len > 0 ? strncmp(s1, s2, len) : 0;
> Example backtrace:
How about a short comment explaining strncmp() is undefined for nullptrs.  Some 
day, we may change things at a lower level so that even when str.len == 0, we 
have a non-null str.ptr, so we can avoid these checks. And so it would be good 
to leave a hint that this branch can be removed at that time.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id563e81720a0a4847664fa2828ecfdcad870da5b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Jim Apple 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5031: remove undefined behavior: call to strncmp with nullptr

2017-04-29 Thread Jim Apple (Code Review) 
Jim Apple has posted comments on this change.

Change subject: IMPALA-5031: remove undefined behavior: call to strncmp with 
nullptr
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6767/1/be/src/runtime/string-value.inline.h
File be/src/runtime/string-value.inline.h:

Line 59:   const int result = len > 0 ? strncmp(s1, s2, len) : 0;
Example backtrace:

/home/ubuntu/Impala/be/src/runtime/string-value.inline.h:60:24: runtime 
error: null pointer passed as argument 1, which is declared to never be null
/usr/include/string.h:148:58: note: nonnull attribute specified here
#0 0x7f72c79c6b6b in impala::StringCompare(char const*, int, char const*, 
int, int) /home/ubuntu/Impala/be/src/runtime/string-value.inline.h:60:16
#1 0x7f72c79c9dba in impala::StringValue::Eq(impala::StringValue const&) 
const /home/ubuntu/Impala/be/src/runtime/string-value.inline.h:82:10
#2 0x7f72c33268a9 in impala::RawValue::Eq(void const*, void const*, 
impala::ColumnType const&) 
/home/ubuntu/Impala/be/src/runtime/raw-value.inline.h:71:14
#3 0x7f72c335f282 in bool 
impala::HashTableCtx::Equals(impala::TupleRow const*, unsigned char 
const*, unsigned char const*) const 
/home/ubuntu/Impala/be/src/exec/hash-table.cc:228:10
#4 0x7f72c33623da in bool 
impala::HashTableCtx::Equals(impala::TupleRow const*) const 
/home/ubuntu/Impala/be/src/exec/hash-table.h:454:12
#5 0x7f72c33623da in long 
impala::HashTable::Probe(impala::HashTable::Bucket*, long, 
impala::HashTableCtx*, unsigned int, bool*) 
/home/ubuntu/Impala/be/src/exec/hash-table.inline.h:71
#6 0x7f72c3ad7beb in 
impala::HashTable::InsertInternal(impala::HashTableCtx*) 
/home/ubuntu/Impala/be/src/exec/hash-table.inline.h:100:24
#7 0x7f72c3ad7568 in impala::HashTable::Insert(impala::HashTableCtx*, 
impala::BufferedTupleStream::RowIdx const&, impala::TupleRow*) 
/home/ubuntu/Impala/be/src/exec/hash-table.inline.h:115:20
#8 0x7f72c3ad69f9 in 
impala::PhjBuilder::Partition::InsertBatch(impala::TPrefetchMode::type, 
impala::HashTableCtx*, impala::RowBatch*, 
std::vector > const&) 
/home/ubuntu/Impala/be/src/exec/partitioned-hash-join-builder-ir.cc:102:35
#9 0x7f72c3aaa402 in impala::PhjBuilder::Partition::BuildHashTable(bool*) 
/home/ubuntu/Impala/be/src/exec/partitioned-hash-join-builder.cc:687:32
#10 0x7f72c3a9c158 in 
impala::PhjBuilder::BuildHashTablesAndPrepareProbeStreams() 
/home/ubuntu/Impala/be/src/exec/partitioned-hash-join-builder.cc:374:31
#11 0x7f72c3a98db4 in impala::PhjBuilder::FlushFinal(impala::RuntimeState*) 
/home/ubuntu/Impala/be/src/exec/partitioned-hash-join-builder.cc:222:29
#12 0x7f72c30dd627 in impala::Status 
impala::BlockingJoinNode::SendBuildInputToSink(impala::RuntimeState*, 
impala::DataSink*) /home/ubuntu/Impala/be/src/exec/blocking-join-node.cc:294:31
#13 0x7f72c30d2fb8 in 
impala::BlockingJoinNode::ProcessBuildInputAsync(impala::RuntimeState*, 
impala::DataSink*, impala::Promise*) 
/home/ubuntu/Impala/be/src/exec/blocking-join-node.cc:154:11
#14 0x7f72c30efaa2 in boost::_mfi::mf3*>::operator()(impala::BlockingJoinNode*, 
impala::RuntimeState*, impala::DataSink*, impala::Promise*) 
const 
/home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/bind/mem_fn_template.hpp:393:16
#15 0x7f72c30ef8ee in void 
boost::_bi::list4, 
boost::_bi::value, boost::_bi::value, 
boost::_bi::value*> 
>::operator()*>, 
boost::_bi::list0>(boost::_bi::type, boost::_mfi::mf3*>&, boost::_bi::list0&, int) 
/home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/bind/bind.hpp:457:9 
 
#16 0x7f72c30ef27b in boost::_bi::bind_t*>, 
boost::_bi::list4, 
boost::_bi::value, boost::_bi::value, 
boost::_bi::value*> > >::operator()() 
/home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/bind/bind_template.hpp:20:16
#17 0x7f72c30ee649 in 
boost::detail::function::void_function_obj_invoker0*>, 
boost::_bi::list4, 
boost::_bi::value, boost::_bi::value, 
boost::_bi::value*> > >, 
void>::invoke(boost::detail::function::function_buffer&) 
/home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/function/function_template.hpp:153:11
#18 0x7f72c798cdd4 in boost::function0::operator()() const 
/home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/function/function_template.hpp:766:14
#19 0x7f72c797df10 in impala::Thread::SuperviseThread(std::string const&, 
std::string const&, boost::function, impala::Promise*) 
/home/ubuntu/Impala/be/src/util/thread.cc:325:3
#20 0x7f72c79a5f83 in void 
boost::_bi::list4, 
boost::_bi::value, boost::_bi::value >, 
boost::_bi::value*> >::operator(), impala::Promise*), 
boost::_bi::list0>(boost::_bi::type, void (*&)(std::string const&, 
std::string const&, boost::function, impala::Promise*), 
boost::_bi::list0&, int) 
/home/ubuntu/Impala/toolchain/boost-1.57.0-p1/include/boost/bind/bind.hpp:457:9
#21 0x7f72c79a58ab in boost::_bi::bind_t, impala::Promise*), 
boost::_bi::lis

[Impala-ASF-CR] IMPALA-5031: remove undefined behavior: call to strncmp with nullptr

2017-04-29 Thread Jim Apple (Code Review) 
Jim Apple has uploaded a new change for review.

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

Change subject: IMPALA-5031: remove undefined behavior: call to strncmp with 
nullptr
..

IMPALA-5031: remove undefined behavior: call to strncmp with nullptr

Calling strncmp with a nullptr is undefined, following paragraph 2 of
section 7.21.1 of the C99 standard (which is included by reference in
C++14). That paragraph states: "Unless explicitly stated otherwise in
the description of a particular function in this subclause, pointer
arguments on such a call shall still have valid values, as described
in 7.1.4." Section 7.1.4 excludes null pointers from "valid values":
"If an argument to a function has an invalid value (such as a value
outside the domain of the function, or a pointer outside the address
space of the program, or a null pointer, or a pointer to
non-modifiable storage when the corresponding parameter is not
const-qualified) or a type (after promotion) not expected by a
function with variable number of arguments, the behavior is
undefined."

I tested this change with string-compare-benchmark.cc and there was no
performance difference.

Change-Id: Id563e81720a0a4847664fa2828ecfdcad870da5b
---
M be/src/benchmarks/string-compare-benchmark.cc
M be/src/runtime/string-value.inline.h
2 files changed, 3 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/67/6767/1
-- 
To view, visit http://gerrit.cloudera.org:8080/6767
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id563e81720a0a4847664fa2828ecfdcad870da5b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple