impala-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mjac...@apache.org
Subject [2/3] incubator-impala git commit: IMPALA-5031: Remove undefined behavior "reference binding to null"
Date Mon, 26 Jun 2017 16:55:41 GMT
IMPALA-5031: Remove undefined behavior "reference binding to null"

When p has type T* and p is nullptr, then T x = p[0] has undefined
behavior (obviously). Less obviously, T& x = p[0] also has undefined
behavior -- references cannot be null. That undefined behavior can be
caused by expressions like &v[0] when v is a vector of size 0. The
issue is that the expression is parsed as &(v[0]), aka
&(v.operator[](0)). The return type of the operator[] method is T&,
and when v is empty the return value is a null reference.

This was recognized by the C++ standards committe and fixed in
Committee Draft 2008. See LWG Defect Report 464:

http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-defects.html#464

This patch was created by running the following command, then fixing
the resulting compile errors:

    find be/src -type f -execdir \
      sed -i 's/&\([A-Za-z0-9_]\+\)\[0\]/\1.data()/g' \{\} \;

Change-Id: Ifef5c3de686318bff4eac7dfad6e1639ddadb8f2
Reviewed-on: http://gerrit.cloudera.org:8080/7008
Reviewed-by: Jim Apple <jbapple-impala@apache.org>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/604759af
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/604759af
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/604759af

Branch: refs/heads/master
Commit: 604759afbc20bdd6b2acdcd5e99f9d9a87b144df
Parents: 00535ab
Author: Jim Apple <jbapple-impala@apache.org>
Authored: Fri May 26 23:20:21 2017 -0700
Committer: Impala Public Jenkins <impala-public-jenkins@gerrit.cloudera.org>
Committed: Sat Jun 24 05:58:56 2017 +0000

----------------------------------------------------------------------
 be/src/exec/delimited-text-parser-test.cc     |  6 +++---
 be/src/exec/hash-table.cc                     |  4 ++--
 be/src/exec/hdfs-parquet-scanner-ir.cc        |  2 +-
 be/src/exec/hdfs-parquet-scanner.cc           |  2 +-
 be/src/exec/hdfs-rcfile-scanner.cc            |  2 +-
 be/src/exec/hdfs-scan-node-base.h             |  2 +-
 be/src/exec/hdfs-scanner.h                    |  3 +--
 be/src/exec/hdfs-sequence-table-writer.cc     | 11 ++++++-----
 be/src/exec/old-hash-table.cc                 |  4 ++--
 be/src/exec/parquet-column-stats.h            |  5 +++--
 be/src/exec/parquet-column-stats.inline.h     |  9 +++++----
 be/src/experiments/tuple-splitter-test.cc     |  2 +-
 be/src/exprs/expr-test.cc                     | 16 ++++++++--------
 be/src/exprs/expr-value.h                     |  2 +-
 be/src/exprs/string-functions-ir.cc           |  5 +++--
 be/src/rpc/authentication.cc                  |  2 +-
 be/src/rpc/thrift-util-test.cc                |  2 +-
 be/src/runtime/bufferpool/buffer-pool-test.cc | 12 ++++++------
 be/src/runtime/disk-io-mgr-test.cc            |  4 ++--
 be/src/runtime/parallel-executor-test.cc      |  2 +-
 be/src/runtime/row-batch-serialize-test.cc    |  2 +-
 be/src/runtime/tmp-file-mgr-test.cc           |  4 ++--
 be/src/udf/udf-test-harness.h                 |  8 ++++----
 be/src/util/bitmap.h                          |  2 +-
 be/src/util/coding-util-test.cc               |  4 ++--
 be/src/util/coding-util.cc                    |  4 ++--
 be/src/util/dict-encoding.h                   |  2 +-
 be/src/util/runtime-profile.cc                |  4 ++--
 be/src/util/streaming-sampler.h               |  4 ++--
 be/src/util/uid-util.h                        |  4 ++--
 be/src/util/webserver.cc                      |  2 +-
 31 files changed, 70 insertions(+), 67 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/exec/delimited-text-parser-test.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/delimited-text-parser-test.cc b/be/src/exec/delimited-text-parser-test.cc
index 4929431..3156b36 100644
--- a/be/src/exec/delimited-text-parser-test.cc
+++ b/be/src/exec/delimited-text-parser-test.cc
@@ -53,9 +53,9 @@ void Validate(DelimitedTextParser* parser, const string& data,
   int num_tuples = 0;
   int num_fields = 0;
   char* next_column_start;
-  Status status = parser->ParseFieldLocations(
-      100, remaining_len, &data_ptr, &row_end_locs[0], &field_locations[0], &num_tuples,
-      &num_fields, &next_column_start);
+  Status status =
+      parser->ParseFieldLocations(100, remaining_len, &data_ptr, &row_end_locs[0],
+          field_locations.data(), &num_tuples, &num_fields, &next_column_start);
   EXPECT_EQ(num_tuples, expected_num_tuples) << data;
   EXPECT_EQ(num_fields, expected_num_fields) << data;
 }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/exec/hash-table.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hash-table.cc b/be/src/exec/hash-table.cc
index 28831ce..0d456f6 100644
--- a/be/src/exec/hash-table.cc
+++ b/be/src/exec/hash-table.cc
@@ -754,7 +754,7 @@ Status HashTableCtx::CodegenEvalRow(LlvmCodeGen* codegen, bool build,
Function**
   Value* expr_values_null = args[3];
   Value* has_null = codegen->false_value();
 
-  // evaluator_vector = &build_expr_evals_[0] / &probe_expr_evals_[0]
+  // evaluator_vector = build_expr_evals_.data() / probe_expr_evals_.data()
   Value* eval_vector = codegen->CodegenCallFunction(&builder, build ?
       IRFunction::HASH_TABLE_GET_BUILD_EXPR_EVALUATORS :
       IRFunction::HASH_TABLE_GET_PROBE_EXPR_EVALUATORS,
@@ -1119,7 +1119,7 @@ Status HashTableCtx::CodegenEquals(LlvmCodeGen* codegen, bool force_null_equalit
   Value* expr_values = args[2];
   Value* expr_values_null = args[3];
 
-  // eval_vector = &build_expr_evals_[0]
+  // eval_vector = build_expr_evals_.data()
   Value* eval_vector = codegen->CodegenCallFunction(&builder,
       IRFunction::HASH_TABLE_GET_BUILD_EXPR_EVALUATORS, this_ptr, "eval_vector");
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/exec/hdfs-parquet-scanner-ir.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-parquet-scanner-ir.cc b/be/src/exec/hdfs-parquet-scanner-ir.cc
index 69951c7..c1574d1 100644
--- a/be/src/exec/hdfs-parquet-scanner-ir.cc
+++ b/be/src/exec/hdfs-parquet-scanner-ir.cc
@@ -27,7 +27,7 @@
 using namespace impala;
 
 int HdfsParquetScanner::ProcessScratchBatch(RowBatch* dst_batch) {
-  ScalarExprEvaluator* const* conjunct_evals = &(*conjunct_evals_)[0];
+  ScalarExprEvaluator* const* conjunct_evals = conjunct_evals_->data();
   const int num_conjuncts = conjunct_evals_->size();
 
   // Start/end/current iterators over the output rows.

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/exec/hdfs-parquet-scanner.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-parquet-scanner.cc b/be/src/exec/hdfs-parquet-scanner.cc
index 6d4b212..8522767 100644
--- a/be/src/exec/hdfs-parquet-scanner.cc
+++ b/be/src/exec/hdfs-parquet-scanner.cc
@@ -1224,7 +1224,7 @@ bool HdfsParquetScanner::AssembleCollection(
       end_of_collection = column_readers[0]->rep_level() <= new_collection_rep_level;
 
       if (materialize_tuple) {
-        if (ExecNode::EvalConjuncts(&evals[0], evals.size(), row)) {
+        if (ExecNode::EvalConjuncts(evals.data(), evals.size(), row)) {
           tuple = next_tuple(tuple_desc->byte_size(), tuple);
           ++num_to_commit;
         }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/exec/hdfs-rcfile-scanner.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-rcfile-scanner.cc b/be/src/exec/hdfs-rcfile-scanner.cc
index 33b62dd..411270a 100644
--- a/be/src/exec/hdfs-rcfile-scanner.cc
+++ b/be/src/exec/hdfs-rcfile-scanner.cc
@@ -311,7 +311,7 @@ Status HdfsRCFileScanner::ReadRowGroupHeader() {
 
 Status HdfsRCFileScanner::ReadKeyBuffers() {
   if (key_buffer_.size() < key_length_) key_buffer_.resize(key_length_);
-  uint8_t* key_buffer = &key_buffer_[0];
+  uint8_t* key_buffer = key_buffer_.data();
 
   if (header_->is_compressed) {
     uint8_t* compressed_buffer;

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/exec/hdfs-scan-node-base.h
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-scan-node-base.h b/be/src/exec/hdfs-scan-node-base.h
index affe2a5..7af5d5d 100644
--- a/be/src/exec/hdfs-scan-node-base.h
+++ b/be/src/exec/hdfs-scan-node-base.h
@@ -182,7 +182,7 @@ class HdfsScanNodeBase : public ScanNode {
   /// The result array is of length hdfs_table_->num_cols(). The i-th element is true
iff
   /// column i should be materialized.
   const bool* is_materialized_col() {
-    return reinterpret_cast<const bool*>(&is_materialized_col_[0]);
+    return reinterpret_cast<const bool*>(is_materialized_col_.data());
   }
 
   /// Returns the per format codegen'd function.  Scanners call this to get the

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/exec/hdfs-scanner.h
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-scanner.h b/be/src/exec/hdfs-scanner.h
index e4a1192..cb9dfeb 100644
--- a/be/src/exec/hdfs-scanner.h
+++ b/be/src/exec/hdfs-scanner.h
@@ -369,8 +369,7 @@ class HdfsScanner {
   /// This must always be inlined so we can correctly replace the call to
   /// ExecNode::EvalConjuncts() during codegen.
   bool IR_ALWAYS_INLINE EvalConjuncts(TupleRow* row)  {
-    return ExecNode::EvalConjuncts(&(*conjunct_evals_)[0],
-        conjunct_evals_->size(), row);
+    return ExecNode::EvalConjuncts(conjunct_evals_->data(), conjunct_evals_->size(),
row);
   }
 
   /// Sets 'num_tuples' template tuples in the batch that 'row' points to. Assumes the

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/exec/hdfs-sequence-table-writer.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/hdfs-sequence-table-writer.cc b/be/src/exec/hdfs-sequence-table-writer.cc
index 37463f3..f8d7b4c 100644
--- a/be/src/exec/hdfs-sequence-table-writer.cc
+++ b/be/src/exec/hdfs-sequence-table-writer.cc
@@ -184,7 +184,8 @@ Status HdfsSequenceTableWriter::WriteCompressedBlock() {
   {
     SCOPED_TIMER(parent_->compress_timer());
     RETURN_IF_ERROR(compressor_->ProcessBlock(false, key_lengths_text.size(),
-        reinterpret_cast<uint8_t*>(&key_lengths_text[0]), &output_length, &output));
+        reinterpret_cast<const uint8_t*>(key_lengths_text.data()), &output_length,
+        &output));
   }
   record.WriteVInt(output_length);
   record.WriteBytes(output_length, output);
@@ -196,7 +197,7 @@ Status HdfsSequenceTableWriter::WriteCompressedBlock() {
   {
     SCOPED_TIMER(parent_->compress_timer());
     RETURN_IF_ERROR(compressor_->ProcessBlock(false, keys_text.size(),
-        reinterpret_cast<uint8_t*>(&keys_text[0]), &output_length, &output));
+        reinterpret_cast<const uint8_t*>(keys_text.data()), &output_length, &output));
   }
   record.WriteVInt(output_length);
   record.WriteBytes(output_length, output);
@@ -206,7 +207,7 @@ Status HdfsSequenceTableWriter::WriteCompressedBlock() {
   {
     SCOPED_TIMER(parent_->compress_timer());
     RETURN_IF_ERROR(compressor_->ProcessBlock(false, value_lengths_text.size(),
-        reinterpret_cast<uint8_t*>(&value_lengths_text[0]), &output_length,
&output));
+        reinterpret_cast<const uint8_t*>(value_lengths_text.data()), &output_length,
&output));
   }
   record.WriteVInt(output_length);
   record.WriteBytes(output_length, output);
@@ -216,7 +217,7 @@ Status HdfsSequenceTableWriter::WriteCompressedBlock() {
   {
     SCOPED_TIMER(parent_->compress_timer());
     RETURN_IF_ERROR(compressor_->ProcessBlock(false, text.size(),
-        reinterpret_cast<uint8_t*>(&text[0]), &output_length, &output));
+        reinterpret_cast<const uint8_t*>(text.data()), &output_length, &output));
   }
   record.WriteVInt(output_length);
   record.WriteBytes(output_length, output);
@@ -299,7 +300,7 @@ inline Status HdfsSequenceTableWriter::ConsumeRow(TupleRow* row) {
     {
       SCOPED_TIMER(parent_->compress_timer());
       RETURN_IF_ERROR(compressor_->ProcessBlock(false, text.size(),
-          reinterpret_cast<uint8_t*>(&text[0]), &value_length, &tmp));
+          reinterpret_cast<const uint8_t*>(text.data()), &value_length, &tmp));
     }
     value_bytes = tmp;
   } else {

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/exec/old-hash-table.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/old-hash-table.cc b/be/src/exec/old-hash-table.cc
index 0050b39..9105226 100644
--- a/be/src/exec/old-hash-table.cc
+++ b/be/src/exec/old-hash-table.cc
@@ -347,7 +347,7 @@ Function* OldHashTable::CodegenEvalTupleRow(LlvmCodeGen* codegen, bool
build) {
   // Aggregation with no grouping exprs also use the hash table interface for
   // code simplicity. In that case, there are no build exprs.
   if (!exprs.empty()) {
-    // Load &build_expr_evals_[0] / &probe_expr_evals_[0]
+    // Load build_expr_evals_.data() / probe_expr_evals_.data()
     Value* eval_vector = codegen->CodegenCallFunction(&builder, build ?
         IRFunction::OLD_HASH_TABLE_GET_BUILD_EXPR_EVALUATORS :
         IRFunction::OLD_HASH_TABLE_GET_PROBE_EXPR_EVALUATORS,
@@ -690,7 +690,7 @@ Function* OldHashTable::CodegenEquals(LlvmCodeGen* codegen) {
   if (!build_exprs_.empty()) {
     BasicBlock* false_block = BasicBlock::Create(context, "false_block", fn);
 
-    // Load &build_expr_evals_[0]
+    // Load build_expr_evals_.data()
     Value* eval_vector = codegen->CodegenCallFunction(&builder,
         IRFunction::OLD_HASH_TABLE_GET_BUILD_EXPR_EVALUATORS,
         this_ptr, "eval_vector");

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/exec/parquet-column-stats.h
----------------------------------------------------------------------
diff --git a/be/src/exec/parquet-column-stats.h b/be/src/exec/parquet-column-stats.h
index c068312..11a01f5 100644
--- a/be/src/exec/parquet-column-stats.h
+++ b/be/src/exec/parquet-column-stats.h
@@ -169,8 +169,9 @@ class ColumnStats : public ColumnStatsBase {
   virtual void EncodeToThrift(parquet::Statistics* out) const override;
 
  protected:
-  /// Encodes a single value using parquet's plain encoding and stores it into the
-  /// binary string 'out'. String values are stored without additional encoding.
+  /// Encodes a single value using parquet's plain encoding and stores it into the binary
+  /// string 'out'. String values are stored without additional encoding. 'bytes_needed'
+  /// must be positive.
   static void EncodePlainValue(const T& v, int64_t bytes_needed, std::string* out);
 
   /// Decodes the plain encoded stats value from 'buffer' and writes the result into the

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/exec/parquet-column-stats.inline.h
----------------------------------------------------------------------
diff --git a/be/src/exec/parquet-column-stats.inline.h b/be/src/exec/parquet-column-stats.inline.h
index 8681436..b112db3 100644
--- a/be/src/exec/parquet-column-stats.inline.h
+++ b/be/src/exec/parquet-column-stats.inline.h
@@ -71,8 +71,9 @@ inline void ColumnStats<T>::EncodeToThrift(parquet::Statistics* out)
const {
 template <typename T>
 inline void ColumnStats<T>::EncodePlainValue(
     const T& v, int64_t bytes_needed, std::string* out) {
+  DCHECK_GT(bytes_needed, 0);
   out->resize(bytes_needed);
-  int64_t bytes_written = ParquetPlainEncoder::Encode(
+  const int64_t bytes_written = ParquetPlainEncoder::Encode(
       v, bytes_needed, reinterpret_cast<uint8_t*>(&(*out)[0]));
   DCHECK_EQ(bytes_needed, bytes_written);
 }
@@ -81,7 +82,7 @@ template <typename T>
 inline bool ColumnStats<T>::DecodePlainValue(const std::string& buffer, void* slot)
{
   T* result = reinterpret_cast<T*>(slot);
   int size = buffer.size();
-  const uint8_t* data = reinterpret_cast<const uint8_t*>(&buffer[0]);
+  const uint8_t* data = reinterpret_cast<const uint8_t*>(buffer.data());
   if (ParquetPlainEncoder::Decode(data, data + size, size, result) == -1) return false;
   return true;
 }
@@ -120,7 +121,7 @@ inline bool ColumnStats<TimestampValue>::DecodePlainValue(
     const std::string& buffer, void* slot) {
   TimestampValue* result = reinterpret_cast<TimestampValue*>(slot);
   int size = buffer.size();
-  const uint8_t* data = reinterpret_cast<const uint8_t*>(&buffer[0]);
+  const uint8_t* data = reinterpret_cast<const uint8_t*>(buffer.data());
   if (ParquetPlainEncoder::Decode(data, data + size, size, result) == -1) return false;
   // We don't need to convert the value here, since we don't support reading timestamp
   // statistics written by Hive / old versions of parquet-mr. Should Hive add support for
@@ -140,7 +141,7 @@ template <>
 inline bool ColumnStats<StringValue>::DecodePlainValue(
     const std::string& buffer, void* slot) {
   StringValue* result = reinterpret_cast<StringValue*>(slot);
-  result->ptr = const_cast<char*>(&buffer[0]);
+  result->ptr = const_cast<char*>(buffer.data());
   result->len = buffer.size();
   return true;
 }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/experiments/tuple-splitter-test.cc
----------------------------------------------------------------------
diff --git a/be/src/experiments/tuple-splitter-test.cc b/be/src/experiments/tuple-splitter-test.cc
index df4fc95..87a52bd 100644
--- a/be/src/experiments/tuple-splitter-test.cc
+++ b/be/src/experiments/tuple-splitter-test.cc
@@ -281,7 +281,7 @@ bool DataPartitioner::Split(const BuildPartition& build_partition)
{
   int partition_mask = partitions - 1;
 
   int new_splits = 0;
-  memset(&split_counts_[0], 0, sizeof(split_counts_[0]) * split_counts_.size());
+  memset(split_counts_.data(), 0, sizeof(split_counts_[0]) * split_counts_.size());
 
   for (int i = 0; i < num_blocks; ++i) {
     int tuples = NumTuples(build_partition, i);

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/exprs/expr-test.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc
index 3b7c3da..462716d 100644
--- a/be/src/exprs/expr-test.cc
+++ b/be/src/exprs/expr-test.cc
@@ -494,15 +494,15 @@ class ExprTest : public testing::Test {
     switch (expected_type.GetByteSize()) {
       case 4:
         EXPECT_EQ(expected_result.value(), StringParser::StringToDecimal<int32_t>(
-            &value[0], value.size(), expected_type, &result).value()) << query;
+            value.data(), value.size(), expected_type, &result).value()) << query;
         break;
       case 8:
         EXPECT_EQ(expected_result.value(), StringParser::StringToDecimal<int64_t>(
-            &value[0], value.size(), expected_type, &result).value()) << query;
+            value.data(), value.size(), expected_type, &result).value()) << query;
         break;
       case 16:
         EXPECT_EQ(expected_result.value(), StringParser::StringToDecimal<int128_t>(
-            &value[0], value.size(), expected_type, &result).value()) << query;
+            value.data(), value.size(), expected_type, &result).value()) << query;
         break;
       default:
         EXPECT_TRUE(false) << expected_type << " " << expected_type.GetByteSize();
@@ -1110,30 +1110,30 @@ bool ExprTest::ConvertValue<bool>(const string& value) {
 template <>
 int8_t ExprTest::ConvertValue<int8_t>(const string& value) {
   StringParser::ParseResult result;
-  return StringParser::StringToInt<int8_t>(&value[0], value.size(), &result);
+  return StringParser::StringToInt<int8_t>(value.data(), value.size(), &result);
 }
 
 template <>
 int16_t ExprTest::ConvertValue<int16_t>(const string& value) {
   StringParser::ParseResult result;
-  return StringParser::StringToInt<int16_t>(&value[0], value.size(), &result);
+  return StringParser::StringToInt<int16_t>(value.data(), value.size(), &result);
 }
 
 template <>
 int32_t ExprTest::ConvertValue<int32_t>(const string& value) {
   StringParser::ParseResult result;
-  return StringParser::StringToInt<int32_t>(&value[0], value.size(), &result);
+  return StringParser::StringToInt<int32_t>(value.data(), value.size(), &result);
 }
 
 template <>
 int64_t ExprTest::ConvertValue<int64_t>(const string& value) {
   StringParser::ParseResult result;
-  return StringParser::StringToInt<int64_t>(&value[0], value.size(), &result);
+  return StringParser::StringToInt<int64_t>(value.data(), value.size(), &result);
 }
 
 template <>
 TimestampValue ExprTest::ConvertValue<TimestampValue>(const string& value) {
-  return TimestampValue::Parse(&value[0], value.size());
+  return TimestampValue::Parse(value.data(), value.size());
 }
 
 // We can't put this into TestValue() because GTest can't resolve

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/exprs/expr-value.h
----------------------------------------------------------------------
diff --git a/be/src/exprs/expr-value.h b/be/src/exprs/expr-value.h
index 5e4eaa8..d53f6c4 100644
--- a/be/src/exprs/expr-value.h
+++ b/be/src/exprs/expr-value.h
@@ -68,8 +68,8 @@ struct ExprValue {
 
   void Init(const std::string& str) {
     string_data = str;
-    string_val.ptr = &string_data[0];
     string_val.len = string_data.size();
+    string_val.ptr = string_val.len > 0 ? &string_data[0] : nullptr;
   }
 
   /// Sets the value for type to '0' and returns a pointer to the data

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/exprs/string-functions-ir.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/string-functions-ir.cc b/be/src/exprs/string-functions-ir.cc
index 09e19fb..5c66541 100644
--- a/be/src/exprs/string-functions-ir.cc
+++ b/be/src/exprs/string-functions-ir.cc
@@ -642,7 +642,7 @@ StringVal StringFunctions::RegexpExtract(FunctionContext* context, const
StringV
   // TODO: fix this
   vector<re2::StringPiece> matches(max_matches);
   bool success =
-      re->Match(str_sp, 0, str.len, re2::RE2::UNANCHORED, &matches[0], max_matches);
+      re->Match(str_sp, 0, str.len, re2::RE2::UNANCHORED, matches.data(), max_matches);
   if (!success) return StringVal();
   // matches[0] is the whole string, matches[1] the first group, etc.
   const re2::StringPiece& match = matches[index.val];
@@ -979,7 +979,8 @@ StringVal StringFunctions::BTrimString(FunctionContext* ctx,
 // Similar to strstr() except that the strings are not null-terminated
 static char* LocateSubstring(char* haystack, int hay_len, const char* needle, int needle_len)
{
   DCHECK_GT(needle_len, 0);
-  DCHECK(haystack != NULL && needle != NULL);
+  DCHECK(needle != NULL);
+  DCHECK(hay_len == 0 || haystack != NULL);
   for (int i = 0; i < hay_len - needle_len + 1; ++i) {
     char* possible_needle = haystack + i;
     if (strncmp(possible_needle, needle, needle_len) == 0) return possible_needle;

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/rpc/authentication.cc
----------------------------------------------------------------------
diff --git a/be/src/rpc/authentication.cc b/be/src/rpc/authentication.cc
index 765ab5d..1a56f7c 100644
--- a/be/src/rpc/authentication.cc
+++ b/be/src/rpc/authentication.cc
@@ -924,7 +924,7 @@ Status SaslAuthProvider::WrapClientTransport(const string& hostname,
   try {
     const string& service = service_name.empty() ? service_name_ : service_name;
     sasl_client.reset(new sasl::TSaslClient(KERBEROS_MECHANISM, auth_id,
-        service, hostname, props, &KERB_INT_CALLBACKS[0]));
+        service, hostname, props, KERB_INT_CALLBACKS.data()));
   } catch (sasl::SaslClientImplException& e) {
     LOG(ERROR) << "Failed to create a GSSAPI/SASL client: " << e.what();
     return Status(e.what());

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/rpc/thrift-util-test.cc
----------------------------------------------------------------------
diff --git a/be/src/rpc/thrift-util-test.cc b/be/src/rpc/thrift-util-test.cc
index ddf1ce9..7193e49 100644
--- a/be/src/rpc/thrift-util-test.cc
+++ b/be/src/rpc/thrift-util-test.cc
@@ -51,7 +51,7 @@ TEST(ThriftUtil, SimpleSerializeDeserialize) {
     EXPECT_OK(serializer.Serialize(&counter, &len1, &buffer1));
 
     EXPECT_EQ(len1, msg.size());
-    EXPECT_TRUE(memcmp(buffer1, &msg[0], len1) == 0);
+    EXPECT_TRUE(memcmp(buffer1, msg.data(), len1) == 0);
 
     // Serialize again and ensure the memory buffer is the same and being reused.
     EXPECT_OK(serializer.Serialize(&counter, &len2, &buffer2));

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/runtime/bufferpool/buffer-pool-test.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/bufferpool/buffer-pool-test.cc b/be/src/runtime/bufferpool/buffer-pool-test.cc
index 1fb60ed..c98ba74 100644
--- a/be/src/runtime/bufferpool/buffer-pool-test.cc
+++ b/be/src/runtime/bufferpool/buffer-pool-test.cc
@@ -1313,7 +1313,7 @@ void BufferPoolTest::TestQueryTeardown(bool write_error) {
     // unpinned pages.
     vector<BufferHandle> tmp_buffers;
     AllocateBuffers(pool, &client, TEST_BUFFER_LEN, CLIENT_BUFFERS, &tmp_buffers);
-    string tmp_file_path = TmpFilePath(&pages[0]);
+    string tmp_file_path = TmpFilePath(pages.data());
     FreeBuffers(pool, &client, &tmp_buffers);
 
     PinAll(pool, &client, &pages);
@@ -1382,7 +1382,7 @@ void BufferPoolTest::TestWriteError(int write_delay_ms) {
   error = pool.CreatePage(&client, TEST_BUFFER_LEN, &tmp_page);
   EXPECT_EQ(TErrorCode::SCRATCH_ALLOCATION_FAILED, error.code());
   EXPECT_FALSE(tmp_page.is_open());
-  error = pool.Pin(&client, &pages[0]);
+  error = pool.Pin(&client, pages.data());
   EXPECT_EQ(TErrorCode::SCRATCH_ALLOCATION_FAILED, error.code());
   EXPECT_FALSE(pages[0].is_pinned());
 
@@ -1416,7 +1416,7 @@ TEST_F(BufferPoolTest, TmpFileAllocateError) {
   vector<PageHandle> pages;
   CreatePages(&pool, &client, TEST_BUFFER_LEN, TOTAL_MEM, &pages);
   // Unpin a page, which will trigger a write.
-  pool.Unpin(&client, &pages[0]);
+  pool.Unpin(&client, pages.data());
   WaitForAllWrites(&client);
   // Remove permissions to the temporary files - subsequent operations will fail.
   ASSERT_GT(RemoveScratchPerms(), 0);
@@ -1425,7 +1425,7 @@ TEST_F(BufferPoolTest, TmpFileAllocateError) {
 
   // Write failure causes future operations like Pin() to fail.
   WaitForAllWrites(&client);
-  Status error = pool.Pin(&client, &pages[0]);
+  Status error = pool.Pin(&client, pages.data());
   EXPECT_EQ(TErrorCode::SCRATCH_ALLOCATION_FAILED, error.code());
   EXPECT_FALSE(pages[0].is_pinned());
 
@@ -1676,7 +1676,7 @@ TEST_F(BufferPoolTest, NoTmpDirs) {
   // active scratch devices.
   UnpinAll(&pool, &client, &pages);
   WaitForAllWrites(&client);
-  ASSERT_OK(pool.Pin(&client, &pages[0]));
+  ASSERT_OK(pool.Pin(&client, pages.data()));
 
   // Allocating another buffer will force a write, which will fail.
   BufferHandle tmp_buffer;
@@ -1718,7 +1718,7 @@ TEST_F(BufferPoolTest, ScratchLimitZero) {
 
   // Spilling is disabled by the QueryState when scratch_limit is 0, so trying to unpin
   // will cause a DCHECK.
-  IMPALA_ASSERT_DEBUG_DEATH(pool->Unpin(&client, &pages[0]), "");
+  IMPALA_ASSERT_DEBUG_DEATH(pool->Unpin(&client, pages.data()), "");
 
   DestroyAll(pool, &client, &pages);
   pool->DeregisterClient(&client);

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/runtime/disk-io-mgr-test.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/disk-io-mgr-test.cc b/be/src/runtime/disk-io-mgr-test.cc
index 38c0dd5..c66fbf1 100644
--- a/be/src/runtime/disk-io-mgr-test.cc
+++ b/be/src/runtime/disk-io-mgr-test.cc
@@ -1074,7 +1074,7 @@ TEST_F(DiskIoMgrTest, ReadIntoClientBuffer) {
     int scan_len = min(len, buffer_len);
     DiskIoMgr::ScanRange* range = AllocateRange(1);
     range->Reset(NULL, tmp_file, scan_len, 0, 0, true,
-        DiskIoMgr::BufferOpts::ReadInto(&client_buffer[0], buffer_len));
+        DiskIoMgr::BufferOpts::ReadInto(client_buffer.data(), buffer_len));
     ASSERT_OK(io_mgr->AddScanRange(reader, range, true));
 
     unique_ptr<DiskIoMgr::BufferDescriptor> io_buffer;
@@ -1112,7 +1112,7 @@ TEST_F(DiskIoMgrTest, ReadIntoClientBufferError) {
     io_mgr->RegisterContext(&reader, reader_mem_tracker);
     DiskIoMgr::ScanRange* range = AllocateRange(1);
     range->Reset(NULL, tmp_file, SCAN_LEN, 0, 0, true,
-        DiskIoMgr::BufferOpts::ReadInto(&client_buffer[0], SCAN_LEN));
+        DiskIoMgr::BufferOpts::ReadInto(client_buffer.data(), SCAN_LEN));
     ASSERT_OK(io_mgr->AddScanRange(reader, range, true));
 
     /// Also test the cancellation path. Run multiple iterations since it is racy whether

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/runtime/parallel-executor-test.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/parallel-executor-test.cc b/be/src/runtime/parallel-executor-test.cc
index c0d797f..8347939 100644
--- a/be/src/runtime/parallel-executor-test.cc
+++ b/be/src/runtime/parallel-executor-test.cc
@@ -72,7 +72,7 @@ TEST(ParallelExecutorTest, Basic) {
 
   EXPECT_OK(ParallelExecutor::Exec(
       bind<Status>(mem_fn(&ParallelExecutorTest::UpdateFunction), &test_caller,
_1),
-      reinterpret_cast<void**>(&args[0]), args.size()));
+      reinterpret_cast<void**>(args.data()), args.size()));
 
   test_caller.Validate();
 }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/runtime/row-batch-serialize-test.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/row-batch-serialize-test.cc b/be/src/runtime/row-batch-serialize-test.cc
index 7a084fc..041e4fe 100644
--- a/be/src/runtime/row-batch-serialize-test.cc
+++ b/be/src/runtime/row-batch-serialize-test.cc
@@ -450,7 +450,7 @@ void RowBatchSerializeTest::TestDupCorrectness(bool full_dedup) {
   RowBatch* batch = pool_.Add(new RowBatch(&row_desc, num_rows, tracker_.get()));
   vector<vector<Tuple*>> distinct_tuples(2);
   CreateTuples(*row_desc.tuple_descriptors()[0], batch->tuple_data_pool(),
-      distinct_int_tuples, 0, 10, &distinct_tuples[0]);
+      distinct_int_tuples, 0, 10, distinct_tuples.data());
   CreateTuples(*row_desc.tuple_descriptors()[1], batch->tuple_data_pool(),
       distinct_string_tuples, 0, 10, &distinct_tuples[1]);
   AddTuplesToRowBatch(num_rows, distinct_tuples, repeats, batch);

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/runtime/tmp-file-mgr-test.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/tmp-file-mgr-test.cc b/be/src/runtime/tmp-file-mgr-test.cc
index dee3c64..c94ba1d 100644
--- a/be/src/runtime/tmp-file-mgr-test.cc
+++ b/be/src/runtime/tmp-file-mgr-test.cc
@@ -470,7 +470,7 @@ TEST_F(TmpFileMgrTest, TestEncryptionDuringCancellation) {
   // Write out a string repeatedly. We don't want to see this written unencypted to disk.
   string plaintext("the quick brown fox jumped over the lazy dog");
   for (int pos = 0; pos + plaintext.size() < DATA_SIZE; pos += plaintext.size()) {
-    memcpy(&data[pos], &plaintext[0], plaintext.size());
+    memcpy(&data[pos], plaintext.data(), plaintext.size());
   }
 
   // Start a write in flight, which should encrypt the data and write it to disk.
@@ -490,7 +490,7 @@ TEST_F(TmpFileMgrTest, TestEncryptionDuringCancellation) {
   FILE* file = fopen(file_path.c_str(), "r");
   ASSERT_EQ(DATA_SIZE, fread(&data[0], 1, DATA_SIZE, file));
   for (int pos = 0; pos + plaintext.size() < DATA_SIZE; pos += plaintext.size()) {
-    ASSERT_NE(0, memcmp(&data[pos], &plaintext[0], plaintext.size()))
+    ASSERT_NE(0, memcmp(&data[pos], plaintext.data(), plaintext.size()))
         << file_path << "@" << pos;
   }
   fclose(file);

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/udf/udf-test-harness.h
----------------------------------------------------------------------
diff --git a/be/src/udf/udf-test-harness.h b/be/src/udf/udf-test-harness.h
index a3cbbdc..fe10b48 100644
--- a/be/src/udf/udf-test-harness.h
+++ b/be/src/udf/udf-test-harness.h
@@ -110,7 +110,7 @@ class UdfTestHarness {
     boost::scoped_ptr<FunctionContext> context(CreateTestContext(return_type, arg_types));
     SetConstantArgs(context.get(), constant_args);
     if (!RunPrepareFn(init_fn, context.get())) return false;
-    RET ret = fn(context.get(), a1.size(), &a1[0]);
+    RET ret = fn(context.get(), a1.size(), a1.data());
     RunCloseFn(close_fn, context.get());
     return Validate(context.get(), expected, ret);
   }
@@ -142,7 +142,7 @@ class UdfTestHarness {
     boost::scoped_ptr<FunctionContext> context(CreateTestContext(return_type, arg_types));
     SetConstantArgs(context.get(), constant_args);
     if (!RunPrepareFn(init_fn, context.get())) return false;
-    RET ret = fn(context.get(), a1, a2.size(), &a2[0]);
+    RET ret = fn(context.get(), a1, a2.size(), a2.data());
     RunCloseFn(close_fn, context.get());
     return Validate(context.get(), expected, ret);
   }
@@ -174,7 +174,7 @@ class UdfTestHarness {
     boost::scoped_ptr<FunctionContext> context(CreateTestContext(return_type, arg_types));
     SetConstantArgs(context.get(), constant_args);
     if (!RunPrepareFn(init_fn, context.get())) return false;
-    RET ret = fn(context.get(), a1, a2, a3.size(), &a3[0]);
+    RET ret = fn(context.get(), a1, a2, a3.size(), a3.data());
     RunCloseFn(close_fn, context.get());
     return Validate(context.get(), expected, ret);
   }
@@ -208,7 +208,7 @@ class UdfTestHarness {
     boost::scoped_ptr<FunctionContext> context(CreateTestContext(return_type, arg_types));
     SetConstantArgs(context.get(), constant_args);
     if (!RunPrepareFn(init_fn, context.get())) return false;
-    RET ret = fn(context.get(), a1, a2, a3, a4.size(), &a4[0]);
+    RET ret = fn(context.get(), a1, a2, a3, a4.size(), a4.data());
     RunCloseFn(close_fn, context.get());
     return Validate(context.get(), expected, ret);
   }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/util/bitmap.h
----------------------------------------------------------------------
diff --git a/be/src/util/bitmap.h b/be/src/util/bitmap.h
index 5f02f60..b2f7f72 100644
--- a/be/src/util/bitmap.h
+++ b/be/src/util/bitmap.h
@@ -75,7 +75,7 @@ class Bitmap {
   }
 
   void SetAllBits(bool b) {
-    memset(&buffer_[0], 255 * b, buffer_.size() * sizeof(uint64_t));
+    memset(buffer_.data(), 255 * b, buffer_.size() * sizeof(uint64_t));
   }
 
   int64_t num_bits() const { return num_bits_; }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/util/coding-util-test.cc
----------------------------------------------------------------------
diff --git a/be/src/util/coding-util-test.cc b/be/src/util/coding-util-test.cc
index 20ca9a8..be479a0 100644
--- a/be/src/util/coding-util-test.cc
+++ b/be/src/util/coding-util-test.cc
@@ -42,7 +42,7 @@ void TestUrl(const string& input, const string& expected_encoded,
bool hive_comp
   // Convert string to vector and try that also
   vector<uint8_t> input_vector;
   input_vector.resize(input.size());
-  memcpy(&input_vector[0], input.c_str(), input.size());
+  memcpy(input_vector.data(), input.c_str(), input.size());
   string intermediate2;
   UrlEncode(input_vector, &intermediate2, hive_compat);
   EXPECT_EQ(intermediate, intermediate2);
@@ -66,7 +66,7 @@ void TestBase64(const string& input, const string& expected_encoded)
{
   // Convert string to vector and try that also
   vector<uint8_t> input_vector;
   input_vector.resize(input.size());
-  memcpy(&input_vector[0], input.c_str(), input.size());
+  memcpy(input_vector.data(), input.c_str(), input.size());
   string intermediate2;
   Base64Encode(input_vector, &intermediate2);
   EXPECT_EQ(intermediate, intermediate2);

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/util/coding-util.cc
----------------------------------------------------------------------
diff --git a/be/src/util/coding-util.cc b/be/src/util/coding-util.cc
index 14fb905..78b9ebd 100644
--- a/be/src/util/coding-util.cc
+++ b/be/src/util/coding-util.cc
@@ -67,7 +67,7 @@ void UrlEncode(const vector<uint8_t>& in, string* out, bool hive_compat)
{
   if (in.empty()) {
     *out = "";
   } else {
-    UrlEncode(reinterpret_cast<const char*>(&in[0]), in.size(), out, hive_compat);
+    UrlEncode(reinterpret_cast<const char*>(in.data()), in.size(), out, hive_compat);
   }
 }
 
@@ -160,7 +160,7 @@ void Base64Encode(const vector<uint8_t>& in, string* out) {
 void Base64Encode(const vector<uint8_t>& in, stringstream* out) {
   if (!in.empty()) {
     // Boost does not like non-null terminated strings
-    string tmp(reinterpret_cast<const char*>(&in[0]), in.size());
+    string tmp(reinterpret_cast<const char*>(in.data()), in.size());
     Base64Encode(tmp.c_str(), tmp.size(), out);
   }
 }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/util/dict-encoding.h
----------------------------------------------------------------------
diff --git a/be/src/util/dict-encoding.h b/be/src/util/dict-encoding.h
index 75e9340..479f095 100644
--- a/be/src/util/dict-encoding.h
+++ b/be/src/util/dict-encoding.h
@@ -309,7 +309,7 @@ ALWAYS_INLINE inline bool DictDecoder<Decimal16Value>::GetNextValue(
   if (index >= dict_.size()) return false;
   // Workaround for IMPALA-959. Use memcpy instead of '=' so addresses
   // do not need to be 16 byte aligned.
-  uint8_t* addr = reinterpret_cast<uint8_t*>(&dict_[0]);
+  uint8_t* addr = reinterpret_cast<uint8_t*>(dict_.data());
   addr = addr + index * sizeof(*value);
   memcpy(value, addr, sizeof(*value));
   return true;

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/util/runtime-profile.cc
----------------------------------------------------------------------
diff --git a/be/src/util/runtime-profile.cc b/be/src/util/runtime-profile.cc
index 5ff9d28..bbc3088 100644
--- a/be/src/util/runtime-profile.cc
+++ b/be/src/util/runtime-profile.cc
@@ -746,8 +746,8 @@ void RuntimeProfile::SerializeToArchiveString(stringstream* out) const
{
   vector<uint8_t> compressed_buffer;
   compressed_buffer.resize(compressor->MaxOutputLen(serialized_buffer.size()));
   int64_t result_len = compressed_buffer.size();
-  uint8_t* compressed_buffer_ptr = &compressed_buffer[0];
-  compressor->ProcessBlock(true, serialized_buffer.size(), &serialized_buffer[0],
+  uint8_t* compressed_buffer_ptr = compressed_buffer.data();
+  compressor->ProcessBlock(true, serialized_buffer.size(), serialized_buffer.data(),
       &result_len, &compressed_buffer_ptr);
   compressed_buffer.resize(result_len);
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/util/streaming-sampler.h
----------------------------------------------------------------------
diff --git a/be/src/util/streaming-sampler.h b/be/src/util/streaming-sampler.h
index 969bdcf..3d254da 100644
--- a/be/src/util/streaming-sampler.h
+++ b/be/src/util/streaming-sampler.h
@@ -52,7 +52,7 @@ class StreamingSampler {
       current_sample_count_(0),
       current_sample_total_time_(0) {
     DCHECK_LE(samples_collected_, MAX_SAMPLES);
-    memcpy(samples_, &initial_samples[0], sizeof(T) * samples_collected_);
+    memcpy(samples_, initial_samples.data(), sizeof(T) * samples_collected_);
   }
 
   /// Add a sample to the sampler. 'ms' is the time elapsed since the last time this
@@ -107,7 +107,7 @@ class StreamingSampler {
     boost::lock_guard<SpinLock> l(lock_);
     period_ = period;
     samples_collected_ = samples.size();
-    memcpy(samples_, &samples[0], sizeof(T) * samples_collected_);
+    memcpy(samples_, samples.data(), sizeof(T) * samples_collected_);
     current_sample_sum_ = 0;
     current_sample_count_ = 0;
     current_sample_total_time_ = 0;

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/util/uid-util.h
----------------------------------------------------------------------
diff --git a/be/src/util/uid-util.h b/be/src/util/uid-util.h
index 1f57298..e20af55 100644
--- a/be/src/util/uid-util.h
+++ b/be/src/util/uid-util.h
@@ -111,8 +111,8 @@ inline string GenerateUUIDString() {
 inline TUniqueId GenerateUUID() {
   const string& u = GenerateUUIDString();
   TUniqueId uid;
-  memcpy(&uid.hi, &u[0], sizeof(int64_t));
-  memcpy(&uid.lo, &u[0] + sizeof(int64_t), sizeof(int64_t));
+  memcpy(&uid.hi, u.data(), sizeof(int64_t));
+  memcpy(&uid.lo, u.data() + sizeof(int64_t), sizeof(int64_t));
   return uid;
 }
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/604759af/be/src/util/webserver.cc
----------------------------------------------------------------------
diff --git a/be/src/util/webserver.cc b/be/src/util/webserver.cc
index 3e587a2..92b6061 100644
--- a/be/src/util/webserver.cc
+++ b/be/src/util/webserver.cc
@@ -329,7 +329,7 @@ Status Webserver::Start() {
   // pointer to this server in the per-server state, and register a static method as the
   // default callback. That method unpacks the pointer to this and calls the real
   // callback.
-  context_ = sq_start(&callbacks, reinterpret_cast<void*>(this), &options[0]);
+  context_ = sq_start(&callbacks, reinterpret_cast<void*>(this), options.data());
 
   // Restore the child signal handler so wait() works properly.
   signal(SIGCHLD, sig_chld);



Mime
View raw message