fix(invocation): resolve issues discovered on a clang build

MakeOut and MethodReturn were relying on undefined behavior on linux builds. Specializations had to be added for certain type combinations, as well as at least one case of MethodReturn needing a corrected return type.
hooklift · Oct 5, 2016 · 00c634d · 00c634d
1 parent 33c03f4
commit 00c634d
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 29 deletions.
diff --git a/src/domain.cc b/src/domain.cc
@@ -239,7 +239,7 @@ void Domain::Initialize(Handle<Object> exports)
 Domain::Domain(virDomainPtr handle) : NLVObject(handle) {}
 
 NAN_METHOD(Domain::LookupByName)
-{  
+{
   Hypervisor::RunMethod<MethodReturnInstance<Domain>>(info, virDomainLookupByName, GetString(info[0]));
 }
 
@@ -391,7 +391,7 @@ NAN_METHOD(Domain::GetOSType)
 
 NAN_METHOD(Domain::GetUUID)
 {
-  RunMethod<MethodReturnString, 2>(info, virDomainGetUUIDString, MakeOutString(VIR_UUID_STRING_BUFLEN));
+  RunMethod<MethodReturn<std::string>, 2>(info, virDomainGetUUIDString, MakeOutString(VIR_UUID_STRING_BUFLEN));
 }
 
 NAN_METHOD(Domain::GetAutostart)

diff --git a/src/nlv_object.h b/src/nlv_object.h
@@ -14,33 +14,33 @@
 using namespace node;
 using namespace v8;
 
-namespace NLV {  
+namespace NLV {
   typedef std::basic_string<unsigned char> ustring;
-  
+
   NAN_INLINE std::string GetString(v8::Local<v8::Value> val) {
     if(!val->IsString()) {
       Nan::ThrowTypeError("argument should be a string");
-    }    
+    }
     return std::string(*Nan::Utf8String(val->ToString()));
   }
-  
+
   NAN_INLINE ustring GetUString(v8::Local<v8::Value> val) {
     if(!val->IsString()) {
       Nan::ThrowTypeError("argument should be a string");
-    }    
+    }
     auto valString = val->ToString();
     return ustring(reinterpret_cast<const unsigned char*>(*Nan::Utf8String(valString)));
   }
-  
-  NAN_INLINE std::shared_ptr<std::string> MakeOutString(size_t siz) {
-    return std::make_shared<std::string>(siz+1, '\0');
+
+  NAN_INLINE std::shared_ptr<std::string> MakeOutString(size_t size) {
+    return std::make_shared<std::string>(size + 1, '\0');
   }
-  
+
   template<typename T, typename ...Args>
   NAN_INLINE std::shared_ptr<T> MakeOut(Args&&... args) {
     return std::make_shared<T>(args...);
   }
-  
+
   NAN_INLINE unsigned int GetFlags(v8::Local<v8::Value> val) {
     if(val->IsUndefined() || val->IsFunction()) {
       return 0;
@@ -58,8 +58,8 @@ namespace NLV {
       return 0;
     }
   }
-    
-  template<typename T, typename Y = T> 
+
+  template<typename T, typename Y = T>
   inline Y&& WrapArg(T&& val) {
     return val;
   }
@@ -69,41 +69,41 @@ namespace NLV {
     inline T operator()(T arg) {
       return arg;
     }
-    
+
     inline const char* operator()(const std::string& arg) {
       return arg.c_str();
     }
-    
+
     inline const unsigned char* operator()(const ustring& arg) {
       return arg.c_str();
     }
-    
+
     inline char* operator()(std::shared_ptr<std::string>& str) {
       return (char*)str->c_str();
     }
-    
+
     template<typename T>
     inline T* operator()(std::shared_ptr<T>& val) {
       return val.get();
     }
   };
-  
+
   struct ReturnUnwrapper {
     template<typename T>
     inline T&& operator()(T&& arg) {
       return arg;
     }
-    
+
     inline char* operator()(std::shared_ptr<std::string>& str) {
       return (char*)str->c_str();
     }
-    
+
     template<typename T>
     inline T operator()(std::shared_ptr<T>& val) {
       return *val;
     }
   };
-    
+
   template <typename ParentClass, typename HandleType, typename CleanupHandler>
   class NLVObject : public NLVObjectBase
   {
@@ -133,21 +133,29 @@ namespace NLV {
       Worker::OnFinishedHandler operator()(T val) {
         return NLV::PrimitiveReturnHandler(static_cast<T>(val));
       }
+
+      Worker::OnFinishedHandler operator()(const std::shared_ptr<std::string> &ptr) {
+        return NLV::PrimitiveReturnHandler(static_cast<T>(*ptr));
+      }
+
+      Worker::OnFinishedHandler operator()(const std::shared_ptr<int> &ptr) {
+        return NLV::PrimitiveReturnHandler(static_cast<T>(*ptr));
+      }
     };
     using MethodReturnString = MethodReturn<const char*>;
-    
+
     template<typename T>
     struct MethodReturnInstance {
       using handle_type = typename T::handle_type;
       Worker::OnFinishedHandler operator()(handle_type handle) {
         return NLV::InstanceReturnHandler<T>(handle);
       }
     };
-    
+
     static inline bool virHasFailed(int val) {
       return val == -1;
     }
-    
+
     static inline bool virHasFailed(const void* ptr) {
       return ptr == nullptr;
     }
@@ -161,11 +169,11 @@ namespace NLV {
       /*
         we're saving all arguments, wrapping them in safely copyable types, like:
         const char* -> std::string
-        
+
         different wrapping methods are defined by specialization/overloading of WrapArg
         - by default WrapArg is just passthrough
       */
-      auto tuple = std::make_tuple(handle, WrapArg(args)...);    
+      auto tuple = std::make_tuple(handle, WrapArg(args)...);
       NLV::Worker::RunAsync(info, [=] (NLV::Worker::SetOnFinishedHandler onFinished) {
         // here we're running in non-v8 thread and the tuple has been copied by value
         // now we need to call virFunction with all the arguments
@@ -178,15 +186,15 @@ namespace NLV {
           return virSaveLastError();
         }
         ReturnHandler returnHandler;
-        
+
         // this tuple contains:
         // 0 - virFunction return value
         // 1 - handle
         // 2 - first arugment after handle
         // by default index 0 will be passed to javascript
         // but by passing ReturnIndex > 1 we can get values returned by pointer
         auto retTuple = std::tuple_cat(std::make_tuple(retVal), tuple);
-        
+
         return onFinished(returnHandler(ReturnUnwrapper()(std::get<ReturnIndex>(retTuple))));
       });
     }

diff --git a/src/worker.h b/src/worker.h
@@ -62,6 +62,15 @@ namespace NLV {
     };
   }
 
+  template<>
+  inline Worker::OnFinishedHandler PrimitiveReturnHandler(std::string val_) {
+    return [=](Worker* worker) {
+      Nan::HandleScope scope;
+      v8::Local<v8::Value> argv[] = { Nan::Null(), Nan::New(val_.c_str()).ToLocalChecked() };
+      worker->Call(2, argv);
+    };
+  }
+
   template<class T, class Y>
   Worker::OnFinishedHandler InstanceReturnHandler(Y val) {
     return [=](Worker* worker) {