You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by we...@apache.org on 2019/06/18 01:33:10 UTC

[arrow] branch master updated: ARROW-5629: [C++] Fix Coverity issues

This is an automated email from the ASF dual-hosted git repository.

wesm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/master by this push:
     new 67f920e  ARROW-5629: [C++] Fix Coverity issues
67f920e is described below

commit 67f920e8dc72c9857daba792014c86b994a4213b
Author: Antoine Pitrou <an...@python.org>
AuthorDate: Mon Jun 17 20:32:59 2019 -0500

    ARROW-5629: [C++] Fix Coverity issues
    
    Nothing serious, just small cleanups.
    
    Note I didn't enable all components when running the analysis, since it's quite long already. I left CUDA, Gandiva, Orc and Plasma disabled.
    
    Author: Antoine Pitrou <an...@python.org>
    
    Closes #4595 from pitrou/ARROW-5629-fix-coverity-issues and squashes the following commits:
    
    fa235f534 <Antoine Pitrou> ARROW-5629:  Fix Coverity issues
---
 cpp/src/arrow/array.h                   | 7 ++++---
 cpp/src/arrow/compare.cc                | 3 ---
 cpp/src/arrow/io/buffered.cc            | 3 ++-
 cpp/src/arrow/io/compressed.cc          | 7 ++++++-
 cpp/src/arrow/json/parser.cc            | 7 +++++--
 cpp/src/arrow/python/flight.cc          | 6 +++---
 cpp/src/arrow/python/flight.h           | 8 +++++---
 cpp/src/arrow/python/helpers.cc         | 2 +-
 cpp/src/arrow/python/python_to_arrow.cc | 2 +-
 cpp/src/arrow/python/serialize.cc       | 1 -
 cpp/src/arrow/util/basic_decimal.cc     | 1 +
 cpp/src/arrow/util/io-util.cc           | 5 +++--
 cpp/src/arrow/util/io-util.h            | 2 +-
 cpp/src/arrow/util/macros.h             | 8 +++++---
 14 files changed, 37 insertions(+), 25 deletions(-)

diff --git a/cpp/src/arrow/array.h b/cpp/src/arrow/array.h
index b3c2539..a655422 100644
--- a/cpp/src/arrow/array.h
+++ b/cpp/src/arrow/array.h
@@ -529,13 +529,14 @@ class ARROW_EXPORT ListArray : public Array {
   }
 
  protected:
-  // this constructor defers SetData to a derived array class
+  // This constructor defers SetData to a derived array class
   ListArray() = default;
   void SetData(const std::shared_ptr<ArrayData>& data);
-  const int32_t* raw_value_offsets_;
+
+  const int32_t* raw_value_offsets_ = NULLPTR;
 
  private:
-  const ListType* list_type_;
+  const ListType* list_type_ = NULLPTR;
   std::shared_ptr<Array> values_;
 };
 
diff --git a/cpp/src/arrow/compare.cc b/cpp/src/arrow/compare.cc
index ca4dfee..12991b9 100644
--- a/cpp/src/arrow/compare.cc
+++ b/cpp/src/arrow/compare.cc
@@ -646,9 +646,6 @@ class ApproxEqualsVisitor : public ArrayEqualsVisitor {
         left, checked_cast<const DoubleArray&>(right_), opts_);
     return Status::OK();
   }
-
- protected:
-  double epsilon_;
 };
 
 static bool BaseDataEquals(const Array& left, const Array& right) {
diff --git a/cpp/src/arrow/io/buffered.cc b/cpp/src/arrow/io/buffered.cc
index 134eb96..ed3cd4e 100644
--- a/cpp/src/arrow/io/buffered.cc
+++ b/cpp/src/arrow/io/buffered.cc
@@ -151,7 +151,8 @@ class BufferedOutputStream::Impl : public BufferedBase {
   }
 
   Status Detach(std::shared_ptr<OutputStream>* raw) {
-    RETURN_NOT_OK(Flush());
+    std::lock_guard<std::mutex> guard(lock_);
+    RETURN_NOT_OK(FlushUnlocked());
     *raw = std::move(raw_);
     is_open_ = false;
     return Status::OK();
diff --git a/cpp/src/arrow/io/compressed.cc b/cpp/src/arrow/io/compressed.cc
index bc04d49..301ebc3 100644
--- a/cpp/src/arrow/io/compressed.cc
+++ b/cpp/src/arrow/io/compressed.cc
@@ -229,7 +229,12 @@ std::shared_ptr<OutputStream> CompressedOutputStream::raw() const { return impl_
 class CompressedInputStream::Impl {
  public:
   Impl(MemoryPool* pool, Codec* codec, const std::shared_ptr<InputStream>& raw)
-      : pool_(pool), raw_(raw), codec_(codec), is_open_(true) {}
+      : pool_(pool),
+        raw_(raw),
+        codec_(codec),
+        is_open_(true),
+        compressed_pos_(0),
+        decompressed_pos_(0) {}
 
   Status Init() {
     RETURN_NOT_OK(codec_->MakeDecompressor(&decompressor_));
diff --git a/cpp/src/arrow/json/parser.cc b/cpp/src/arrow/json/parser.cc
index 5e51f84..6e85628 100644
--- a/cpp/src/arrow/json/parser.cc
+++ b/cpp/src/arrow/json/parser.cc
@@ -214,7 +214,7 @@ class RawArrayBuilder<Kind::kBoolean> {
 class ScalarBuilder {
  public:
   explicit ScalarBuilder(MemoryPool* pool)
-      : data_builder_(pool), null_bitmap_builder_(pool) {}
+      : values_length_(0), data_builder_(pool), null_bitmap_builder_(pool) {}
 
   Status Append(int32_t index, int32_t value_length) {
     RETURN_NOT_OK(data_builder_.Append(index));
@@ -567,7 +567,10 @@ class HandlerBase : public BlockParser,
                     public rj::BaseReaderHandler<rj::UTF8<>, HandlerBase> {
  public:
   explicit HandlerBase(MemoryPool* pool)
-      : BlockParser(pool), builder_set_(pool), scalar_values_builder_(pool) {}
+      : BlockParser(pool),
+        builder_set_(pool),
+        field_index_(-1),
+        scalar_values_builder_(pool) {}
 
   /// Retrieve a pointer to a builder from a BuilderPtr
   template <Kind::type kind>
diff --git a/cpp/src/arrow/python/flight.cc b/cpp/src/arrow/python/flight.cc
index 409ba60..ee19fb9 100644
--- a/cpp/src/arrow/python/flight.cc
+++ b/cpp/src/arrow/python/flight.cc
@@ -30,7 +30,7 @@ namespace py {
 namespace flight {
 
 PyServerAuthHandler::PyServerAuthHandler(PyObject* handler,
-                                         PyServerAuthHandlerVtable vtable)
+                                         const PyServerAuthHandlerVtable& vtable)
     : vtable_(vtable) {
   Py_INCREF(handler);
   handler_.reset(handler);
@@ -53,7 +53,7 @@ Status PyServerAuthHandler::IsValid(const std::string& token,
 }
 
 PyClientAuthHandler::PyClientAuthHandler(PyObject* handler,
-                                         PyClientAuthHandlerVtable vtable)
+                                         const PyClientAuthHandlerVtable& vtable)
     : vtable_(vtable) {
   Py_INCREF(handler);
   handler_.reset(handler);
@@ -74,7 +74,7 @@ Status PyClientAuthHandler::GetToken(std::string* token) {
   });
 }
 
-PyFlightServer::PyFlightServer(PyObject* server, PyFlightServerVtable vtable)
+PyFlightServer::PyFlightServer(PyObject* server, const PyFlightServerVtable& vtable)
     : vtable_(vtable) {
   Py_INCREF(server);
   server_.reset(server);
diff --git a/cpp/src/arrow/python/flight.h b/cpp/src/arrow/python/flight.h
index 432885c..aecb97a 100644
--- a/cpp/src/arrow/python/flight.h
+++ b/cpp/src/arrow/python/flight.h
@@ -80,7 +80,8 @@ class ARROW_PYTHON_EXPORT PyClientAuthHandlerVtable {
 /// \brief A helper to implement an auth mechanism in Python.
 class ARROW_PYTHON_EXPORT PyServerAuthHandler : public arrow::flight::ServerAuthHandler {
  public:
-  explicit PyServerAuthHandler(PyObject* handler, PyServerAuthHandlerVtable vtable);
+  explicit PyServerAuthHandler(PyObject* handler,
+                               const PyServerAuthHandlerVtable& vtable);
   Status Authenticate(arrow::flight::ServerAuthSender* outgoing,
                       arrow::flight::ServerAuthReader* incoming) override;
   Status IsValid(const std::string& token, std::string* peer_identity) override;
@@ -93,7 +94,8 @@ class ARROW_PYTHON_EXPORT PyServerAuthHandler : public arrow::flight::ServerAuth
 /// \brief A helper to implement an auth mechanism in Python.
 class ARROW_PYTHON_EXPORT PyClientAuthHandler : public arrow::flight::ClientAuthHandler {
  public:
-  explicit PyClientAuthHandler(PyObject* handler, PyClientAuthHandlerVtable vtable);
+  explicit PyClientAuthHandler(PyObject* handler,
+                               const PyClientAuthHandlerVtable& vtable);
   Status Authenticate(arrow::flight::ClientAuthSender* outgoing,
                       arrow::flight::ClientAuthReader* incoming) override;
   Status GetToken(std::string* token) override;
@@ -105,7 +107,7 @@ class ARROW_PYTHON_EXPORT PyClientAuthHandler : public arrow::flight::ClientAuth
 
 class ARROW_PYTHON_EXPORT PyFlightServer : public arrow::flight::FlightServerBase {
  public:
-  explicit PyFlightServer(PyObject* server, PyFlightServerVtable vtable);
+  explicit PyFlightServer(PyObject* server, const PyFlightServerVtable& vtable);
 
   // Like Serve(), but set up signals and invoke Python signal handlers
   // if necessary.  This function may return with a Python exception set.
diff --git a/cpp/src/arrow/python/helpers.cc b/cpp/src/arrow/python/helpers.cc
index 28ed1a6..362bcd1 100644
--- a/cpp/src/arrow/python/helpers.cc
+++ b/cpp/src/arrow/python/helpers.cc
@@ -120,13 +120,13 @@ std::string PyObject_StdStringRepr(PyObject* obj) {
   }
 #else
   OwnedRef bytes_ref(PyObject_Repr(obj));
+#endif
   if (!bytes_ref) {
     PyErr_Clear();
     std::stringstream ss;
     ss << "<object of type '" << Py_TYPE(obj)->tp_name << "' repr() failed>";
     return ss.str();
   }
-#endif
   return PyBytes_AsStdString(bytes_ref.obj());
 }
 
diff --git a/cpp/src/arrow/python/python_to_arrow.cc b/cpp/src/arrow/python/python_to_arrow.cc
index 2d1d5d2..1e1c59a 100644
--- a/cpp/src/arrow/python/python_to_arrow.cc
+++ b/cpp/src/arrow/python/python_to_arrow.cc
@@ -827,7 +827,7 @@ class DecimalConverter : public TypedConverter<arrow::Decimal128Type, DecimalCon
   }
 
  private:
-  const DecimalType* decimal_type_;
+  const DecimalType* decimal_type_ = nullptr;
 };
 
 #define NUMERIC_CONVERTER(TYPE_ENUM, TYPE)                           \
diff --git a/cpp/src/arrow/python/serialize.cc b/cpp/src/arrow/python/serialize.cc
index 669fd0a..8ff0e01 100644
--- a/cpp/src/arrow/python/serialize.cc
+++ b/cpp/src/arrow/python/serialize.cc
@@ -341,7 +341,6 @@ Status CallCustomCallback(PyObject* context, PyObject* method_name, PyObject* el
     *result = PyObject_CallMethodObjArgs(context, method_name, elem, NULL);
     return PassPyError();
   }
-  return Status::OK();
 }
 
 Status CallSerializeCallback(PyObject* context, PyObject* value,
diff --git a/cpp/src/arrow/util/basic_decimal.cc b/cpp/src/arrow/util/basic_decimal.cc
index 75fb3db..d88e624 100644
--- a/cpp/src/arrow/util/basic_decimal.cc
+++ b/cpp/src/arrow/util/basic_decimal.cc
@@ -456,6 +456,7 @@ DecimalStatus BasicDecimal128::Divide(const BasicDecimal128& divisor,
 
   int64_t result_length = dividend_length - divisor_length;
   uint32_t result_array[4];
+  DCHECK_LE(result_length, 4);
 
   // Normalize by shifting both by a multiple of 2 so that
   // the digit guessing is better. The requirement is that
diff --git a/cpp/src/arrow/util/io-util.cc b/cpp/src/arrow/util/io-util.cc
index 2dcd05d..3b95ca4 100644
--- a/cpp/src/arrow/util/io-util.cc
+++ b/cpp/src/arrow/util/io-util.cc
@@ -888,7 +888,7 @@ Status TemporaryDir::Make(const std::string& prefix, std::unique_ptr<TemporaryDi
   return Status::OK();
 }
 
-SignalHandler::SignalHandler() {}
+SignalHandler::SignalHandler() : SignalHandler(static_cast<Callback>(nullptr)) {}
 
 SignalHandler::SignalHandler(Callback cb) {
 #if ARROW_HAVE_SIGACTION
@@ -939,7 +939,8 @@ Status GetSignalHandler(int signum, SignalHandler* out) {
   return Status::OK();
 }
 
-Status SetSignalHandler(int signum, SignalHandler handler, SignalHandler* old_handler) {
+Status SetSignalHandler(int signum, const SignalHandler& handler,
+                        SignalHandler* old_handler) {
 #if ARROW_HAVE_SIGACTION
   struct sigaction old_sa;
   int ret = sigaction(signum, &handler.action(), &old_sa);
diff --git a/cpp/src/arrow/util/io-util.h b/cpp/src/arrow/util/io-util.h
index 2b48a5c..dd6ff57 100644
--- a/cpp/src/arrow/util/io-util.h
+++ b/cpp/src/arrow/util/io-util.h
@@ -254,7 +254,7 @@ class ARROW_EXPORT SignalHandler {
 ARROW_EXPORT
 Status GetSignalHandler(int signum, SignalHandler* out);
 ARROW_EXPORT
-Status SetSignalHandler(int signum, SignalHandler handler,
+Status SetSignalHandler(int signum, const SignalHandler& handler,
                         SignalHandler* old_handler = NULLPTR);
 
 }  // namespace internal
diff --git a/cpp/src/arrow/util/macros.h b/cpp/src/arrow/util/macros.h
index 4516985..8b92bce 100644
--- a/cpp/src/arrow/util/macros.h
+++ b/cpp/src/arrow/util/macros.h
@@ -81,7 +81,11 @@
 // clang-format off
 // [[deprecated]] is only available in C++14, use this for the time being
 // This macro takes an optional deprecation message
-#if __cplusplus <= 201103L
+#ifdef __COVERITY__
+#  define ARROW_DEPRECATED(...)
+#elif __cplusplus > 201103L
+#  define ARROW_DEPRECATED(...) [[deprecated(__VA_ARGS__)]]
+#else
 # ifdef __GNUC__
 #  define ARROW_DEPRECATED(...) __attribute__((deprecated(__VA_ARGS__)))
 # elif defined(_MSC_VER)
@@ -89,8 +93,6 @@
 # else
 #  define ARROW_DEPRECATED(...)
 # endif
-#else
-#  define ARROW_DEPRECATED(...) [[deprecated(__VA_ARGS__)]]
 #endif
 
 // ----------------------------------------------------------------------