Coverity fixes, round one

src/fs_event_wrap.cc

@@ -34,25 +34,25 @@ class FSEventWrap: public HandleWrap {
   size_t self_size() const override { return sizeof(*this); }
 
  private:
+  static const encoding kDefaultEncoding = UTF8;
+
   FSEventWrap(Environment* env, Local<Object> object);
   ~FSEventWrap() override;
 
   static void OnEvent(uv_fs_event_t* handle, const char* filename, int events,
     int status);
 
   uv_fs_event_t handle_;
-  bool initialized_;
-  enum encoding encoding_;
+  bool initialized_ = false;
+  enum encoding encoding_ = kDefaultEncoding;
 };
 
 
 FSEventWrap::FSEventWrap(Environment* env, Local<Object> object)
     : HandleWrap(env,
                  object,
                  reinterpret_cast<uv_handle_t*>(&handle_),
-                 AsyncWrap::PROVIDER_FSEVENTWRAP) {
-  initialized_ = false;
-}
+                 AsyncWrap::PROVIDER_FSEVENTWRAP) {}
 
 
 FSEventWrap::~FSEventWrap() {
@@ -88,6 +88,7 @@ void FSEventWrap::Start(const FunctionCallbackInfo<Value>& args) {
 
   FSEventWrap* wrap;
   ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
+  CHECK_EQ(wrap->initialized_, false);
 
   static const char kErrMsg[] = "filename must be a string or Buffer";
   if (args.Length() < 1)
@@ -101,7 +102,7 @@ void FSEventWrap::Start(const FunctionCallbackInfo<Value>& args) {
   if (args[2]->IsTrue())
     flags |= UV_FS_EVENT_RECURSIVE;
 
-  wrap->encoding_ = ParseEncoding(env->isolate(), args[3], UTF8);
+  wrap->encoding_ = ParseEncoding(env->isolate(), args[3], kDefaultEncoding);
 
   int err = uv_fs_event_init(wrap->env()->event_loop(), &wrap->handle_);
   if (err == 0) {

src/node.cc

@@ -4096,9 +4096,9 @@ void Init(int* argc,
 
   // init async debug messages dispatching
   // Main thread uses uv_default_loop
-  uv_async_init(uv_default_loop(),
-                &dispatch_debug_messages_async,
-                DispatchDebugMessagesAsyncCallback);
+  CHECK_EQ(0, uv_async_init(uv_default_loop(),
+                            &dispatch_debug_messages_async,
+                            DispatchDebugMessagesAsyncCallback));
   uv_unref(reinterpret_cast<uv_handle_t*>(&dispatch_debug_messages_async));
 
 #if defined(NODE_V8_OPTIONS)

src/node_crypto.cc

@@ -3585,19 +3585,16 @@ void Hmac::New(const FunctionCallbackInfo<Value>& args) {
 void Hmac::HmacInit(const char* hash_type, const char* key, int key_len) {
   HandleScope scope(env()->isolate());
 
-  CHECK_EQ(md_, nullptr);
-  md_ = EVP_get_digestbyname(hash_type);
-  if (md_ == nullptr) {
+  CHECK_EQ(initialised_, false);
+  const EVP_MD* md = EVP_get_digestbyname(hash_type);
+  if (md == nullptr) {
     return env()->ThrowError("Unknown message digest");
   }
   HMAC_CTX_init(&ctx_);
-  int result = 0;
   if (key_len == 0) {
-    result = HMAC_Init(&ctx_, "", 0, md_);
-  } else {
-    result = HMAC_Init(&ctx_, key, key_len, md_);
+    key = "";
   }
-  if (!result) {
+  if (!HMAC_Init_ex(&ctx_, key, key_len, md, nullptr)) {
     return ThrowCryptoError(env(), ERR_get_error());
   }
   initialised_ = true;
@@ -3730,12 +3727,12 @@ void Hash::New(const FunctionCallbackInfo<Value>& args) {
 
 
 bool Hash::HashInit(const char* hash_type) {
-  CHECK_EQ(md_, nullptr);
-  md_ = EVP_get_digestbyname(hash_type);
-  if (md_ == nullptr)
+  CHECK_EQ(initialised_, false);
+  const EVP_MD* md = EVP_get_digestbyname(hash_type);
+  if (md == nullptr)
     return false;
   EVP_MD_CTX_init(&mdctx_);
-  if (EVP_DigestInit_ex(&mdctx_, md_, nullptr) <= 0) {
+  if (EVP_DigestInit_ex(&mdctx_, md, nullptr) <= 0) {
     return false;
   }
   initialised_ = true;
@@ -3881,13 +3878,13 @@ void Sign::New(const FunctionCallbackInfo<Value>& args) {
 
 
 SignBase::Error Sign::SignInit(const char* sign_type) {
-  CHECK_EQ(md_, nullptr);
-  md_ = EVP_get_digestbyname(sign_type);
-  if (!md_)
+  CHECK_EQ(initialised_, false);
+  const EVP_MD* md = EVP_get_digestbyname(sign_type);
+  if (md == nullptr)
     return kSignUnknownDigest;
 
   EVP_MD_CTX_init(&mdctx_);
-  if (!EVP_SignInit_ex(&mdctx_, md_, nullptr))
+  if (!EVP_SignInit_ex(&mdctx_, md, nullptr))
     return kSignInit;
   initialised_ = true;
 
@@ -4087,13 +4084,13 @@ void Verify::New(const FunctionCallbackInfo<Value>& args) {
 
 
 SignBase::Error Verify::VerifyInit(const char* verify_type) {
-  CHECK_EQ(md_, nullptr);
-  md_ = EVP_get_digestbyname(verify_type);
-  if (md_ == nullptr)
+  CHECK_EQ(initialised_, false);
+  const EVP_MD* md = EVP_get_digestbyname(verify_type);
+  if (md == nullptr)
     return kSignUnknownDigest;
 
   EVP_MD_CTX_init(&mdctx_);
-  if (!EVP_VerifyInit_ex(&mdctx_, md_, nullptr))
+  if (!EVP_VerifyInit_ex(&mdctx_, md, nullptr))
     return kSignInit;
   initialised_ = true;
 

src/node_crypto.h

@@ -500,14 +500,12 @@ class Hmac : public BaseObject {
 
   Hmac(Environment* env, v8::Local<v8::Object> wrap)
       : BaseObject(env, wrap),
-        md_(nullptr),
         initialised_(false) {
     MakeWeak<Hmac>(this);
   }
 
  private:
   HMAC_CTX ctx_; /* coverity[member_decl] */
-  const EVP_MD* md_; /* coverity[member_decl] */
   bool initialised_;
 };
 
@@ -531,14 +529,12 @@ class Hash : public BaseObject {
 
   Hash(Environment* env, v8::Local<v8::Object> wrap)
       : BaseObject(env, wrap),
-        md_(nullptr),
         initialised_(false) {
     MakeWeak<Hash>(this);
   }
 
  private:
   EVP_MD_CTX mdctx_; /* coverity[member_decl] */
-  const EVP_MD* md_; /* coverity[member_decl] */
   bool initialised_;
   bool finalized_;
 };
@@ -557,7 +553,6 @@ class SignBase : public BaseObject {
 
   SignBase(Environment* env, v8::Local<v8::Object> wrap)
       : BaseObject(env, wrap),
-        md_(nullptr),
         initialised_(false) {
   }
 
@@ -571,7 +566,6 @@ class SignBase : public BaseObject {
   void CheckThrow(Error error);
 
   EVP_MD_CTX mdctx_; /* coverity[member_decl] */
-  const EVP_MD* md_; /* coverity[member_decl] */
   bool initialised_;
 };
 

src/node_file.cc

@@ -1079,38 +1079,23 @@ static void WriteBuffers(const FunctionCallbackInfo<Value>& args) {
   int64_t pos = GET_OFFSET(args[2]);
   Local<Value> req = args[3];
 
-  uint32_t chunkCount = chunks->Length();
+  MaybeStackBuffer<uv_buf_t> iovs(chunks->Length());
 
-  uv_buf_t s_iovs[1024];  // use stack allocation when possible
-  uv_buf_t* iovs;
-
-  if (chunkCount > arraysize(s_iovs))
-    iovs = new uv_buf_t[chunkCount];
-  else
-    iovs = s_iovs;
-
-  for (uint32_t i = 0; i < chunkCount; i++) {
+  for (uint32_t i = 0; i < iovs.length(); i++) {
     Local<Value> chunk = chunks->Get(i);
 
-    if (!Buffer::HasInstance(chunk)) {
-      if (iovs != s_iovs)
-        delete[] iovs;
+    if (!Buffer::HasInstance(chunk))
       return env->ThrowTypeError("Array elements all need to be buffers");
-    }
 
     iovs[i] = uv_buf_init(Buffer::Data(chunk), Buffer::Length(chunk));
   }
 
   if (req->IsObject()) {
-    ASYNC_CALL(write, req, UTF8, fd, iovs, chunkCount, pos)
-    if (iovs != s_iovs)
-      delete[] iovs;
+    ASYNC_CALL(write, req, UTF8, fd, *iovs, iovs.length(), pos)
     return;
   }
 
-  SYNC_CALL(write, nullptr, fd, iovs, chunkCount, pos)
-  if (iovs != s_iovs)
-    delete[] iovs;
+  SYNC_CALL(write, nullptr, fd, *iovs, iovs.length(), pos)
   args.GetReturnValue().Set(SYNC_RESULT);
 }
 

src/node_win32_etw_provider.cc

@@ -155,9 +155,9 @@ void init_etw() {
     event_write = (EventWriteFunc)GetProcAddress(advapi, "EventWrite");
 
     // create async object used to invoke main thread from callback
-    uv_async_init(uv_default_loop(),
-                  &dispatch_etw_events_change_async,
-                  etw_events_change_async);
+    CHECK_EQ(0, uv_async_init(uv_default_loop(),
+                              &dispatch_etw_events_change_async,
+                              etw_events_change_async));
     uv_unref(reinterpret_cast<uv_handle_t*>(&dispatch_etw_events_change_async));
 
     if (event_register) {

src/node_zlib.cc

@@ -149,8 +149,7 @@ class ZCtx : public AsyncWrap {
 
     if (args[1]->IsNull()) {
       // just a flush
-      Bytef nada[1] = { 0 };
-      in = nada;
+      in = nullptr;
       in_len = 0;
       in_off = 0;
     } else {

src/process_wrap.cc

@@ -123,25 +123,19 @@ class ProcessWrap : public HandleWrap {
     // options.uid
     Local<Value> uid_v = js_options->Get(env->uid_string());
     if (uid_v->IsInt32()) {
-      int32_t uid = uid_v->Int32Value();
-      if (uid & ~((uv_uid_t) ~0)) {
-        return env->ThrowRangeError("options.uid is out of range");
-      }
+      const int32_t uid = uid_v->Int32Value(env->context()).FromJust();
       options.flags |= UV_PROCESS_SETUID;
-      options.uid = (uv_uid_t) uid;
+      options.uid = static_cast<uv_uid_t>(uid);
     } else if (!uid_v->IsUndefined() && !uid_v->IsNull()) {
       return env->ThrowTypeError("options.uid should be a number");
     }
 
     // options.gid
     Local<Value> gid_v = js_options->Get(env->gid_string());
     if (gid_v->IsInt32()) {
-      int32_t gid = gid_v->Int32Value();
-      if (gid & ~((uv_gid_t) ~0)) {
-        return env->ThrowRangeError("options.gid is out of range");
-      }
+      const int32_t gid = gid_v->Int32Value(env->context()).FromJust();
       options.flags |= UV_PROCESS_SETGID;
-      options.gid = (uv_gid_t) gid;
+      options.gid = static_cast<uv_gid_t>(gid);
     } else if (!gid_v->IsUndefined() && !gid_v->IsNull()) {
       return env->ThrowTypeError("options.gid should be a number");
     }

src/spawn_sync.cc

@@ -729,17 +729,19 @@ int SyncProcessRunner::ParseOptions(Local<Value> js_value) {
   }
   Local<Value> js_uid = js_options->Get(env()->uid_string());
   if (IsSet(js_uid)) {
-    if (!CheckRange<uv_uid_t>(js_uid))
+    if (!js_uid->IsInt32())
       return UV_EINVAL;
-    uv_process_options_.uid = static_cast<uv_gid_t>(js_uid->Int32Value());
+    const int32_t uid = js_uid->Int32Value(env()->context()).FromJust();
+    uv_process_options_.uid = static_cast<uv_uid_t>(uid);
     uv_process_options_.flags |= UV_PROCESS_SETUID;
   }
 
   Local<Value> js_gid = js_options->Get(env()->gid_string());
   if (IsSet(js_gid)) {
-    if (!CheckRange<uv_gid_t>(js_gid))
+    if (!js_gid->IsInt32())
       return UV_EINVAL;
-    uv_process_options_.gid = static_cast<uv_gid_t>(js_gid->Int32Value());
+    const int32_t gid = js_gid->Int32Value(env()->context()).FromJust();
+    uv_process_options_.gid = static_cast<uv_gid_t>(gid);
     uv_process_options_.flags |= UV_PROCESS_SETGID;
   }
 
@@ -763,7 +765,7 @@ int SyncProcessRunner::ParseOptions(Local<Value> js_value) {
 
   Local<Value> js_max_buffer = js_options->Get(env()->max_buffer_string());
   if (IsSet(js_max_buffer)) {
-    if (!CheckRange<uint32_t>(js_max_buffer))
+    if (!js_max_buffer->IsUint32())
       return UV_EINVAL;
     max_buffer_ = js_max_buffer->Uint32Value();
   }
@@ -915,27 +917,6 @@ bool SyncProcessRunner::IsSet(Local<Value> value) {
 }
 
 
-template <typename t>
-bool SyncProcessRunner::CheckRange(Local<Value> js_value) {
-  if ((t) -1 > 0) {
-    // Unsigned range check.
-    if (!js_value->IsUint32())
-      return false;
-    if (js_value->Uint32Value() & ~((t) ~0))
-      return false;
-
-  } else {
-    // Signed range check.
-    if (!js_value->IsInt32())
-      return false;
-    if (js_value->Int32Value() & ~((t) ~0))
-      return false;
-  }
-
-  return true;
-}
-
-
 int SyncProcessRunner::CopyJsString(Local<Value> js_value,
                                     const char** target) {
   Isolate* isolate = env()->isolate();

src/spawn_sync.h

@@ -175,7 +175,6 @@ class SyncProcessRunner {
   inline int AddStdioInheritFD(uint32_t child_fd, int inherit_fd);
 
   static bool IsSet(Local<Value> value);
-  template <typename t> static bool CheckRange(Local<Value> js_value);
   int CopyJsString(Local<Value> js_value, const char** target);
   int CopyJsStringArray(Local<Value> js_value, char** target);
 

src/tls_wrap.h

@@ -150,7 +150,6 @@ class TLSWrap : public AsyncWrap,
   BIO* enc_out_;
   NodeBIO* clear_in_;
   size_t write_size_;
-  size_t write_queue_size_;
   typedef ListHead<WriteItem, &WriteItem::member_> WriteItemList;
   WriteItemList write_item_queue_;
   WriteItemList pending_write_items_;

src/util.h

@@ -231,6 +231,16 @@ class MaybeStackBuffer {
     return buf_;
   }
 
+  T& operator[](size_t index) {
+    CHECK_LT(index, length());
+    return buf_[index];
+  }
+
+  const T& operator[](size_t index) const {
+    CHECK_LT(index, length());
+    return buf_[index];
+  }
+
   size_t length() const {
     return length_;
   }
@@ -282,6 +292,10 @@ class MaybeStackBuffer {
     buf_[0] = T();
   }
 
+  explicit MaybeStackBuffer(size_t storage) : MaybeStackBuffer() {
+    AllocateSufficientStorage(storage);
+  }
+
   ~MaybeStackBuffer() {
     if (buf_ != buf_st_)
       free(buf_);