nodegit · johnhaley81 · Apr 11, 2016 · Apr 6, 2016
diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json
@@ -1307,7 +1307,7 @@
       "ignore": true
     },
     "oid": {
-      "dupFunction": "git_oid_cpy",
+      "cpyFunction": "git_oid_cpy",
       "freeFunctionName": "free",
       "shouldAlloc": true,
       "functions": {

diff --git a/generate/scripts/generateNativeCode.js b/generate/scripts/generateNativeCode.js
@@ -35,7 +35,8 @@ module.exports = function generateNativeCode() {
     fields: utils.readFile("templates/partials/fields.cc"),
     guardArguments: utils.readFile("templates/partials/guard_arguments.cc"),
     syncFunction: utils.readFile("templates/partials/sync_function.cc"),
-    fieldAccessors: utils.readFile("templates/partials/field_accessors.cc")
+    fieldAccessors: utils.readFile("templates/partials/field_accessors.cc"),
+    traits: utils.readFile("templates/partials/traits.h")
   };
 
   var templates = {
@@ -87,7 +88,9 @@ module.exports = function generateNativeCode() {
 
   // Attach all partials to select templates.
   Object.keys(partials).forEach(function(partial) {
+    templates.class_header.registerPartial(partial, combyne(partials[partial]));
     templates.class_content.registerPartial(partial, combyne(partials[partial]));
+    templates.struct_header.registerPartial(partial, combyne(partials[partial]));
     templates.struct_content.registerPartial(partial, combyne(partials[partial]));
   });
 

diff --git a/generate/scripts/helpers.js b/generate/scripts/helpers.js
@@ -183,6 +183,8 @@ var Helpers = {
 
     if (typeDefOverrides.freeFunctionName) {
       typeDef.freeFunctionName = typeDefOverrides.freeFunctionName;
+    } else if (typeDef.type === 'struct') {
+      typeDef.freeFunctionName = 'free';
     }
 
     typeDef.fields = typeDef.fields || [];

diff --git a/generate/templates/manual/include/nodegit_wrapper.h b/generate/templates/manual/include/nodegit_wrapper.h
@@ -0,0 +1,64 @@
+#ifndef NODEGIT_WRAPPER_H
+#define NODEGIT_WRAPPER_H
+
+#include <nan.h>
+
+// the Traits template parameter supplies:
+//  typename cppClass - the C++ type of the NodeGit wrapper (e.g. GitRepository)
+//  typename cType - the C type of the libgit2 object being wrapped (e.g. git_repository)
+//
+//  static const bool isDuplicable
+//  static void duplicate(cType **dest, cType *src) - duplicates src using dupFunction or cpyFunction
+//
+//  static const bool isFreeable
+//  static void free(cType *raw) - frees the object using freeFunctionName
+
+template<typename Traits>
+class NodeGitWrapper : public Nan::ObjectWrap {
+public:
+  // replicate Traits typedefs for ease of use
+  typedef typename Traits::cType cType;
+  typedef typename Traits::cppClass cppClass;
+
+  // whether raw should be freed on destruction
+  // TODO: this should be protected but we have a few use cases that change this to
+  // false from the outside.  I suspect it gets turned to false to avoid
+  // double-free problems in cases like when we pass cred objects to libgit2
+  // and it frees them.  We should probably be NULLing raw in that case
+  // (and through a method) instead of changing selfFreeing, but that's
+  // a separate issue.
+  bool selfFreeing;
+protected:
+  cType *raw;
+
+  // owner of the object, in the memory management sense. only populated
+  // when using ownedByThis, and the type doesn't have a dupFunction
+  // CopyablePersistentTraits are used to get the reset-on-destruct behavior.
+  Nan::Persistent<v8::Object, Nan::CopyablePersistentTraits<v8::Object> > owner;
+
+  static Nan::Persistent<v8::Function> constructor_template;
+
+  // diagnostic count of self-freeing object instances
+  static int SelfFreeingInstanceCount;
+  // diagnostic count of constructed non-self-freeing object instances
+  static int NonSelfFreeingConstructedCount;
+
+  static void InitializeTemplate(v8::Local<v8::FunctionTemplate> &tpl);
+
+  NodeGitWrapper(cType *raw, bool selfFreeing, v8::Local<v8::Object> owner);
+  NodeGitWrapper(const char *error); // calls ThrowError
+  ~NodeGitWrapper();
+
+  static NAN_METHOD(JSNewFunction);
+
+  static NAN_METHOD(GetSelfFreeingInstanceCount);
+  static NAN_METHOD(GetNonSelfFreeingConstructedCount);
+
+public:
+  static v8::Local<v8::Value> New(const cType *raw, bool selfFreeing, v8::Local<v8::Object> owner = v8::Local<v8::Object>());
+
+  cType *GetValue();
+  void ClearValue();
+};
+
+#endif
diff --git a/generate/templates/manual/src/nodegit_wrapper.cc b/generate/templates/manual/src/nodegit_wrapper.cc
@@ -0,0 +1,113 @@
+template<typename Traits>
+NodeGitWrapper<Traits>::NodeGitWrapper(typename Traits::cType *raw, bool selfFreeing, v8::Local<v8::Object> owner) {
+  if (!owner.IsEmpty()) {
+    // if we have an owner, there are two options - either we duplicate the raw object
+    // (so we own the duplicate, and can self-free it)
+    // or we keep a handle on the owner so it doesn't get garbage collected
+    // while this wrapper is accessible
+    if(Traits::isDuplicable) {
+      Traits::duplicate(&this->raw, raw);
+      selfFreeing = true;
+    } else {
+      this->owner.Reset(owner);
+      this->raw = raw;
+    }
+  } else {
+    this->raw = raw;
+  }
+  this->selfFreeing = selfFreeing;
+
+  if (selfFreeing) {
+    SelfFreeingInstanceCount++;
+  } else {
+    NonSelfFreeingConstructedCount++;
+  }
+}
+
+template<typename Traits>
+NodeGitWrapper<Traits>::NodeGitWrapper(const char *error) {
+  selfFreeing = false;
+  raw = NULL;
+  Nan::ThrowError(error);
+}
+
+template<typename Traits>
+NodeGitWrapper<Traits>::~NodeGitWrapper() {
+  if(Traits::isFreeable && selfFreeing) {
+    Traits::free(raw);
+    SelfFreeingInstanceCount--;
+    raw = NULL;
+  }
+}
+
+template<typename Traits>
+NAN_METHOD(NodeGitWrapper<Traits>::JSNewFunction) {
+  cppClass * instance;
+
+  if (info.Length() == 0 || !info[0]->IsExternal()) {
+    Nan::TryCatch tryCatch;
+    instance = new cppClass();
+    // handle the case where the default constructor is not supported
+    if(tryCatch.HasCaught()) {
+      delete instance;
+      tryCatch.ReThrow();
+      return;
+    }
+  } else {
+    instance = new cppClass(static_cast<cType *>(
+      Local<External>::Cast(info[0])->Value()),
+      Nan::To<bool>(info[1]).FromJust(),
+      info.Length() >= 3 && !info[2].IsEmpty() && info[2]->IsObject() ? info[2]->ToObject() : Local<v8::Object>()
+    );
+  }
+
+  instance->Wrap(info.This());
+  info.GetReturnValue().Set(info.This());
+}
+
+template<typename Traits>
+v8::Local<v8::Value> NodeGitWrapper<Traits>::New(const typename Traits::cType *raw, bool selfFreeing, v8::Local<v8::Object> owner) {
+  Nan::EscapableHandleScope scope;
+  Local<v8::Value> argv[3] = { Nan::New<External>((void *)raw), Nan::New(selfFreeing), owner };
+  return scope.Escape(
+    Nan::NewInstance(
+      Nan::New(constructor_template),
+      owner.IsEmpty() ? 2 : 3, // passing an empty handle as part of the arguments causes a crash
+      argv
+    ).ToLocalChecked());
+}
+
+template<typename Traits>
+typename Traits::cType *NodeGitWrapper<Traits>::GetValue() {
+  return raw;
+}
+
+template<typename Traits>
+void NodeGitWrapper<Traits>::ClearValue() {
+  raw = NULL;
+}
+
+template<typename Traits>
+Nan::Persistent<v8::Function> NodeGitWrapper<Traits>::constructor_template;
+
+template<typename Traits>
+int NodeGitWrapper<Traits>::SelfFreeingInstanceCount;
+
+template<typename Traits>
+int NodeGitWrapper<Traits>::NonSelfFreeingConstructedCount;
+
+template<typename Traits>
+NAN_METHOD(NodeGitWrapper<Traits>::GetSelfFreeingInstanceCount) {
+  info.GetReturnValue().Set(SelfFreeingInstanceCount);
+}
+
+template<typename Traits>
+NAN_METHOD(NodeGitWrapper<Traits>::GetNonSelfFreeingConstructedCount) {
+  info.GetReturnValue().Set(NonSelfFreeingConstructedCount);
+}
+
+template<typename Traits>
+void NodeGitWrapper<Traits>::InitializeTemplate(v8::Local<v8::FunctionTemplate> &tpl) {
+  Nan::SetMethod(tpl, "getSelfFreeingInstanceCount", GetSelfFreeingInstanceCount);
+  Nan::SetMethod(tpl, "getNonSelfFreeingConstructedCount", GetNonSelfFreeingConstructedCount);
+}
diff --git a/generate/templates/partials/traits.h b/generate/templates/partials/traits.h
@@ -0,0 +1,28 @@
+class {{ cppClassName }};
+
+struct {{ cppClassName }}Traits {
+  typedef {{ cppClassName }} cppClass;
+  typedef {{ cType }} cType;
+
+  static const bool isDuplicable = {{ dupFunction|toBool |or cpyFunction|toBool}};
+  static void duplicate({{ cType }} **dest, {{ cType }} *src) {
+  {% if dupFunction %}
+    {{ dupFunction }}(dest, src);
+  {% elsif cpyFunction %}
+    {{ cType }} *copy = ({{ cType }} *)malloc(sizeof({{ cType }}));
+    {{ cpyFunction }}(copy, src);
+    *dest = copy;
+  {% else %}
+    Nan::ThrowError("duplicate called on {{ cppClassName }} which cannot be duplicated");
+  {% endif %}
+  }
+
+  static const bool isFreeable = {{ freeFunctionName | toBool}};
+  static void free({{ cType }} *raw) {
+  {% if freeFunctionName %}
+    ::{{ freeFunctionName }}(raw); // :: to avoid calling this free recursively
+  {% else %}
+    Nan::ThrowError("free called on {{ cppClassName }} which cannot be freed");
+  {% endif %}
+  }
+};
diff --git a/generate/templates/templates/class_content.cc b/generate/templates/templates/class_content.cc
@@ -11,6 +11,7 @@ extern "C" {
 #include "../include/lock_master.h"
 #include "../include/functions/copy.h"
 #include "../include/{{ filename }}.h"
+#include "nodegit_wrapper.cc"
 
 {% each dependencies as dependency %}
   #include "{{ dependency }}"
@@ -23,47 +24,7 @@ using namespace v8;
 using namespace node;
 
 {% if cType %}
-  {{ cppClassName }}::{{ cppClassName }}({{ cType }} *raw, bool selfFreeing, Local<v8::Object> owner) {
-    if (!owner.IsEmpty()) {
-      // if we have an owner, there are two options - either we duplicate the raw object
-      // (so we own the duplicate, and can self-free it)
-      // or we keep a handle on the owner so it doesn't get garbage collected
-      // while this wrapper is accessible
-      {% if dupFunction %}
-        {% if shouldAlloc %}
-      this->raw = ({{ cType }} *)malloc(sizeof({{ cType }}));
-      {{ dupFunction }}(this->raw, raw);
-        {% else %}
-      {{ dupFunction }}(&this->raw, raw);
-        {% endif %}
-      selfFreeing = true;
-      {% else %}
-      this->owner.Reset(owner);
-      this->raw = raw;
-      {% endif %}
-    } else {
-      this->raw = raw;
-    }
-    this->selfFreeing = selfFreeing;
-
-    if (selfFreeing) {
-      SelfFreeingInstanceCount++;
-    } else {
-      NonSelfFreeingConstructedCount++;
-    }
-
-  }
-
   {{ cppClassName }}::~{{ cppClassName }}() {
-    {% if freeFunctionName %}
-      if (this->selfFreeing) {
-        {{ freeFunctionName }}(this->raw);
-        SelfFreeingInstanceCount--;
-
-        this->raw = NULL;
-      }
-    {% endif %}
-
     // this will cause an error if you have a non-self-freeing object that also needs
     // to save values. Since the object that will eventually free the object has no
     // way of knowing to free these values.
@@ -104,61 +65,13 @@ using namespace node;
       {% endif %}
     {% endeach %}
 
-    Nan::SetMethod(tpl, "getSelfFreeingInstanceCount", GetSelfFreeingInstanceCount);
-    Nan::SetMethod(tpl, "getNonSelfFreeingConstructedCount", GetNonSelfFreeingConstructedCount);
+    InitializeTemplate(tpl);
 
     Local<Function> _constructor_template = Nan::GetFunction(tpl).ToLocalChecked();
     constructor_template.Reset(_constructor_template);
     Nan::Set(target, Nan::New("{{ jsClassName }}").ToLocalChecked(), _constructor_template);
   }
 
-  NAN_METHOD({{ cppClassName }}::JSNewFunction) {
-
-    if (info.Length() == 0 || !info[0]->IsExternal()) {
-      {% if createFunctionName %}
-        return Nan::ThrowError("A new {{ cppClassName }} cannot be instantiated. Use {{ jsCreateFunctionName }} instead.");
-      {% else %}
-        return Nan::ThrowError("A new {{ cppClassName }} cannot be instantiated.");
-      {% endif %}
-    }
-
-    {{ cppClassName }}* object = new {{ cppClassName }}(static_cast<{{ cType }} *>(
-      Local<External>::Cast(info[0])->Value()),
-      Nan::To<bool>(info[1]).FromJust(),
-      info.Length() >= 3 && !info[2].IsEmpty() && info[2]->IsObject() ? info[2]->ToObject() : Local<v8::Object>()
-    );
-    object->Wrap(info.This());
-
-    info.GetReturnValue().Set(info.This());
-  }
-
-  Local<v8::Value> {{ cppClassName }}::New(const {{ cType }} *raw, bool selfFreeing, Local<v8::Object> owner) {
-    Nan::EscapableHandleScope scope;
-    Local<v8::Value> argv[3] = { Nan::New<External>((void *)raw), Nan::New(selfFreeing), owner };
-    return scope.Escape(
-      Nan::NewInstance(
-        Nan::New({{ cppClassName }}::constructor_template),
-        owner.IsEmpty() ? 2 : 3, // passing an empty handle as part of the arguments causes a crash
-        argv
-      ).ToLocalChecked());
-  }
-
-  NAN_METHOD({{ cppClassName }}::GetSelfFreeingInstanceCount) {
-    info.GetReturnValue().Set(SelfFreeingInstanceCount);
-  }
-
-  NAN_METHOD({{ cppClassName }}::GetNonSelfFreeingConstructedCount) {
-    info.GetReturnValue().Set(NonSelfFreeingConstructedCount);
-  }
-
-  {{ cType }} *{{ cppClassName }}::GetValue() {
-    return this->raw;
-  }
-
-  void {{ cppClassName }}::ClearValue() {
-    this->raw = NULL;
-  }
-
 {% else %}
 
   void {{ cppClassName }}::InitializeComponent(Local<v8::Object> target) {
@@ -191,9 +104,8 @@ using namespace node;
 
 {% partial fields . %}
 
-{% if not cTypeIsUndefined %}
-  Nan::Persistent<Function> {{ cppClassName }}::constructor_template;
+{%if cType %}
+// force base class template instantiation, to make sure we get all the
+// methods, statics, etc.
+template class NodeGitWrapper<{{ cppClassName }}Traits>;
 {% endif %}
-
-int {{ cppClassName }}::SelfFreeingInstanceCount;
-int {{ cppClassName }}::NonSelfFreeingConstructedCount;