From b37d1c9c1f055e5f08ad20f71d9ade9f25ac03f1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= <cmn@dwim.me>
Date: Thu, 27 Mar 2014 13:03:42 +0100
Subject: [PATCH] Plug memory leaks

This patch is mostly about making sure that we free the copies of what
we have, as well as making sure that we can free it.

The IndexEntry forgot to free its path, but it also used a pointer to
python-owned memory, which could be freed at any time.

MergeResult completely lacked a deallocator.

Signature needs to make sure we can free the enocoding, and not to set
an owner when we own the memory (in this case for the default
signature).

The repository needs to get rid of its reference to the object list when
returning.

The transfer progress callback needs to decref the stats object.
---
 src/index.c       |  6 ++++--
 src/mergeresult.c | 10 +++++++++-
 src/remote.c      |  1 +
 src/repository.c  |  8 ++++++--
 src/signature.c   | 30 +++++++++++++++++++++---------
 src/types.h       |  2 +-
 6 files changed, 42 insertions(+), 15 deletions(-)

diff --git a/src/index.c b/src/index.c
index cc1b365..bc72c59 100644
--- a/src/index.c
+++ b/src/index.c
@@ -623,8 +623,9 @@ IndexEntry_init(IndexEntry *self, PyObject *args, PyObject *kwds)
         return -1;
 
     memset(&self->entry, 0, sizeof(struct git_index_entry));
-    if (c_path)
-        self->entry.path = c_path;
+    self->entry.path = strdup(c_path);
+    if (!self->entry.path)
+        return -1;
 
     if (id)
         git_oid_cpy(&self->entry.oid, &id->oid);
@@ -638,6 +639,7 @@ IndexEntry_init(IndexEntry *self, PyObject *args, PyObject *kwds)
 void
 IndexEntry_dealloc(IndexEntry *self)
 {
+    free(self->entry.path);
     PyObject_Del(self);
 }
 
diff --git a/src/mergeresult.c b/src/mergeresult.c
index ada872f..345e56e 100644
--- a/src/mergeresult.c
+++ b/src/mergeresult.c
@@ -50,6 +50,14 @@ git_merge_result_to_python(git_merge_result *merge_result)
     return (PyObject*) py_merge_result;
 }
 
+void
+MergeResult_dealloc(MergeResult *self)
+{
+    git_merge_result_free(self->result);
+    PyObject_Del(self);
+}
+
+
 PyDoc_STRVAR(MergeResult_is_uptodate__doc__, "Is up to date");
 
 PyObject *
@@ -99,7 +107,7 @@ PyTypeObject MergeResultType = {
     "_pygit2.MergeResult",                     /* tp_name           */
     sizeof(MergeResult),                       /* tp_basicsize      */
     0,                                         /* tp_itemsize       */
-    0,                                         /* tp_dealloc        */
+    (destructor)MergeResult_dealloc,           /* tp_dealloc        */
     0,                                         /* tp_print          */
     0,                                         /* tp_getattr        */
     0,                                         /* tp_setattr        */
diff --git a/src/remote.c b/src/remote.c
index c738784..379584d 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -175,6 +175,7 @@ transfer_progress_cb(const git_transfer_progress *stats, void *data)
         return -1;
 
     ret = PyObject_CallFunctionObjArgs(remote->transfer_progress, py_stats, NULL);
+    Py_DECREF(py_stats);
     if (!ret)
         return -1;
 
diff --git a/src/repository.c b/src/repository.c
index 031e6e0..b10dd58 100644
--- a/src/repository.c
+++ b/src/repository.c
@@ -139,6 +139,7 @@ Repository_as_iter(Repository *self)
     git_odb *odb;
     int err;
     PyObject *accum = PyList_New(0);
+    PyObject *ret;
 
     err = git_repository_odb(&odb, self->repo);
     if (err < 0)
@@ -151,7 +152,10 @@ Repository_as_iter(Repository *self)
     if (err < 0)
         return Error_set(err);
 
-    return PyObject_GetIter(accum);
+    ret = PyObject_GetIter(accum);
+    Py_DECREF(accum);
+
+    return ret;
 }
 
 
@@ -1345,7 +1349,7 @@ Repository_default_signature__get__(Repository *self)
     if ((err = git_signature_default(&sig, self->repo)) < 0)
         return Error_set(err);
 
-    return build_signature((Object*) self, sig, "utf-8");
+    return build_signature(NULL, sig, "utf-8");
 }
 
 PyDoc_STRVAR(Repository_checkout_head__doc__,
diff --git a/src/signature.c b/src/signature.c
index 809e802..727178d 100644
--- a/src/signature.c
+++ b/src/signature.c
@@ -83,11 +83,12 @@ Signature_init(Signature *self, PyObject *args, PyObject *kwds)
 void
 Signature_dealloc(Signature *self)
 {
-    if (self->obj)
+    /* self->obj is the owner of the git_signature, so we musn't free it */
+    if (self->obj) {
         Py_CLEAR(self->obj);
-    else {
-        git_signature_free((git_signature*)self->signature);
-        free((char*)self->encoding);
+    } else {
+        git_signature_free((git_signature *) self->signature);
+        free(self->encoding);
     }
 
     PyObject_Del(self);
@@ -224,12 +225,23 @@ build_signature(Object *obj, const git_signature *signature,
     Signature *py_signature;
 
     py_signature = PyObject_New(Signature, &SignatureType);
+    if (!py_signature)
+        goto on_error;
 
-    if (py_signature) {
-        Py_INCREF(obj);
-        py_signature->obj = obj;
-        py_signature->signature = signature;
-        py_signature->encoding = encoding;
+    py_signature->encoding = NULL;
+    if (encoding) {
+        py_signature->encoding = strdup(encoding);
+        if (!py_signature->encoding)
+            goto on_error;
     }
+
+    Py_XINCREF(obj);
+    py_signature->obj = obj;
+    py_signature->signature = signature;
+
     return (PyObject*)py_signature;
+
+on_error:
+    git_signature_free((git_signature *) signature);
+    return NULL;
 }
diff --git a/src/types.h b/src/types.h
index e6a3189..3e704a7 100644
--- a/src/types.h
+++ b/src/types.h
@@ -191,7 +191,7 @@ typedef struct {
     PyObject_HEAD
     Object *obj;
     const git_signature *signature;
-    const char *encoding;
+    char *encoding;
 } Signature;