diff --git a/include/pygit2/config.h b/include/pygit2/config.h index 976ee3d..b0eba93 100644 --- a/include/pygit2/config.h +++ b/include/pygit2/config.h @@ -32,6 +32,7 @@ #include #include +PyObject* wrap_config(char *c_path); PyObject* Config_get_global_config(void); PyObject* Config_get_system_config(void); PyObject* Config_add_file(Config *self, PyObject *args, PyObject *kwds); @@ -39,6 +40,6 @@ PyObject* Config_getitem(Config *self, PyObject *key); PyObject* Config_foreach(Config *self, PyObject *args); PyObject* Config_get_multivar(Config *self, PyObject *args); PyObject* Config_set_multivar(Config *self, PyObject *args); +int Config_init(Config *self, PyObject *args, PyObject *kwds); int Config_setitem(Config *self, PyObject *key, PyObject *value); - #endif diff --git a/include/pygit2/types.h b/include/pygit2/types.h index dfd08c0..27bccda 100644 --- a/include/pygit2/types.h +++ b/include/pygit2/types.h @@ -57,22 +57,31 @@ OBJECT_STRUCT(Blob, git_blob, blob) OBJECT_STRUCT(Tag, git_tag, tag) OBJECT_STRUCT(Index, git_index, index) OBJECT_STRUCT(Walker, git_revwalk, walk) -OBJECT_STRUCT(Config, git_config, config) OBJECT_STRUCT(Remote, git_remote, remote) OBJECT_STRUCT(Diff, git_diff_list, list) typedef struct { PyObject_HEAD - git_diff_list* list; + git_config* config; +} Config; + +typedef struct { + PyObject_HEAD + Diff* diff; size_t i; size_t n; } DiffIter; typedef struct { PyObject_HEAD - PyObject* files; PyObject* hunks; -} DiffEntry; + const char * old_file_path; + const char * new_file_path; + char* old_oid; + char* new_oid; + unsigned status; + unsigned similarity; +} Patch; typedef struct { PyObject_HEAD @@ -82,18 +91,11 @@ typedef struct { typedef struct { PyObject_HEAD - const char *header; + PyObject* lines; int old_start; int old_lines; - char* old_oid; - int old_mode; - const char* old_file; int new_start; int new_lines; - char* new_oid; - int new_mode; - const char* new_file; - PyObject *data; } Hunk; typedef struct { @@ -128,7 +130,6 @@ typedef struct { typedef struct { PyObject_HEAD - Reference *reference; git_reflog *reflog; int i; int size; diff --git a/include/pygit2/utils.h b/include/pygit2/utils.h index 161b194..d1ecc42 100644 --- a/include/pygit2/utils.h +++ b/include/pygit2/utils.h @@ -95,9 +95,6 @@ char * py_str_to_c_str(PyObject *value, const char *encoding); #define py_path_to_c_str(py_path) \ py_str_to_c_str(py_path, Py_FileSystemDefaultEncoding) -#define INSTANCIATE_CLASS(type, arglist) \ - PyObject_CallObject(PyType_GenericNew(&type, NULL, NULL), arglist); - /* Helpers to make shorter PyMethodDef and PyGetSetDef blocks */ #define METHOD(type, name, args)\ {#name, (PyCFunction) type ## _ ## name, args, type ## _ ## name ## __doc__} @@ -119,21 +116,22 @@ char * py_str_to_c_str(PyObject *value, const char *encoding); #define MEMBER(type, attr, attr_type, docstr)\ {#attr, attr_type, offsetof(type, attr), 0, PyDoc_STR(docstr)} + /* Helpers for memory allocation */ - - -#define MALLOC(ptr, size, label) \ - ptr = realloc(ptr, size * sizeof(char));\ +#define CALLOC(ptr, num, size, label) \ + ptr = calloc((num), size);\ if (ptr == NULL) {\ err = GIT_ERROR;\ giterr_set_oom();\ goto label;\ - }\ - -#define FREE(to_free)\ - if (to_free != NULL) { free(to_free); to_free = NULL; } -#define FREE_FUNC(to_free, fnct)\ - if (to_free != NULL) { fnct(to_free); to_free = NULL; } + } +#define MALLOC(ptr, size, label) \ + ptr = malloc(size);\ + if (ptr == NULL) {\ + err = GIT_ERROR;\ + giterr_set_oom();\ + goto label;\ + } #endif diff --git a/src/config.c b/src/config.c index b2167b6..9cf0b55 100644 --- a/src/config.c +++ b/src/config.c @@ -34,10 +34,28 @@ extern PyTypeObject ConfigType; + +PyObject * +wrap_config(char *c_path) { + int err; + PyObject *py_path; + Config *py_config; + + py_path = Py_BuildValue("(s)", c_path); + py_config = PyObject_New(Config, &ConfigType); + + err = Config_init(py_config, py_path, NULL); + if (err < 0) + return NULL; + + return (PyObject*) py_config; +} + + int Config_init(Config *self, PyObject *args, PyObject *kwds) { - char *path; + char *path = NULL; int err; if (kwds) { @@ -46,18 +64,17 @@ Config_init(Config *self, PyObject *args, PyObject *kwds) return -1; } - if (PySequence_Length(args) > 0) { - if (!PyArg_ParseTuple(args, "s", &path)) { - return -1; - } + if (!PyArg_ParseTuple(args, "|s", &path)) + return -1; + if (path == NULL) + err = git_config_new(&self->config); + else err = git_config_open_ondisk(&self->config, path); - } else { - err = git_config_new(&self->config); - } - if (err < 0) { + git_config_free(self->config); + if (err == GIT_ENOTFOUND) { Error_set_exc(PyExc_IOError); } else { @@ -70,35 +87,14 @@ Config_init(Config *self, PyObject *args, PyObject *kwds) return 0; } + void Config_dealloc(Config *self) { - PyObject_GC_UnTrack(self); - Py_XDECREF(self->repo); git_config_free(self->config); - PyObject_GC_Del(self); + PyObject_Del(self); } -int -Config_traverse(Config *self, visitproc visit, void *arg) -{ - Py_VISIT(self->repo); - return 0; -} - -PyObject * -Config_open(char *c_path) { - PyObject *py_path = Py_BuildValue("(s)", c_path); - Config *config = PyObject_GC_New(Config, &ConfigType); - - Config_init(config, py_path, NULL); - - Py_INCREF(config); - - return (PyObject *)config; -} - - PyDoc_STRVAR(Config_get_global_config__doc__, "get_global_config() -> Config\n" "\n" @@ -116,10 +112,11 @@ Config_get_global_config(void) PyErr_SetString(PyExc_IOError, "Global config file not found."); return NULL; } + return Error_set(err); } - return Config_open(path); + return wrap_config(path); } @@ -143,9 +140,10 @@ Config_get_system_config(void) return Error_set(err); } - return Config_open(path); + return wrap_config(path); } + int Config_contains(Config *self, PyObject *py_key) { int err; @@ -158,9 +156,11 @@ Config_contains(Config *self, PyObject *py_key) { err = git_config_get_string(&c_value, self->config, c_key); free(c_key); - if (err == GIT_ENOTFOUND) - return 0; + if (err < 0) { + if (err == GIT_ENOTFOUND) + return 0; + Error_set(err); return -1; } @@ -168,69 +168,71 @@ Config_contains(Config *self, PyObject *py_key) { return 1; } + PyObject * Config_getitem(Config *self, PyObject *py_key) { - int err; - int64_t c_intvalue; - int c_boolvalue; - const char *c_charvalue; - char *c_key; + long value_int; + int err, value_bool; + const char *value_str; + char *key; + PyObject* py_value; - if (!(c_key = py_str_to_c_str(py_key, NULL))) + key = py_str_to_c_str(py_key, NULL); + if (key == NULL) return NULL; - err = git_config_get_int64(&c_intvalue, self->config, c_key); - if (err == GIT_OK) { - free(c_key); - return PyInt_FromLong((long)c_intvalue); - } + err = git_config_get_string(&value_str, self->config, key); + if (err < 0) + goto cleanup; - err = git_config_get_bool(&c_boolvalue, self->config, c_key); - if (err == GIT_OK) { - free(c_key); - return PyBool_FromLong((long)c_boolvalue); - } + if (git_config_parse_int64(&value_int, value_str) == 0) + py_value = PyLong_FromLong(value_int); + else if(git_config_parse_bool(&value_bool, value_str) == 0) + py_value = PyBool_FromLong(value_bool); + else + py_value = PyUnicode_FromString(value_str); + +cleanup: + free(key); - err = git_config_get_string(&c_charvalue, self->config, c_key); - free(c_key); if (err < 0) { if (err == GIT_ENOTFOUND) { PyErr_SetObject(PyExc_KeyError, py_key); return NULL; } + return Error_set(err); } - return PyUnicode_FromString(c_charvalue); + return py_value; } int Config_setitem(Config *self, PyObject *py_key, PyObject *py_value) { int err; - char *c_key; - char *py_str; + char *key, *value; - if (!(c_key = py_str_to_c_str(py_key, NULL))) + key = py_str_to_c_str(py_key, NULL); + if (key == NULL) return -1; - if (!py_value) { - err = git_config_delete_entry(self->config, c_key); - } else if (PyBool_Check(py_value)) { - err = git_config_set_bool(self->config, c_key, + if (py_value == NULL) + err = git_config_delete_entry(self->config, key); + else if (PyBool_Check(py_value)) { + err = git_config_set_bool(self->config, key, (int)PyObject_IsTrue(py_value)); } else if (PyInt_Check(py_value)) { - err = git_config_set_int64(self->config, c_key, + err = git_config_set_int64(self->config, key, (int64_t)PyInt_AsLong(py_value)); } else { - py_value = PyObject_Str(py_value); - py_str = py_str_to_c_str(py_value, NULL); - err = git_config_set_string(self->config, c_key, py_str); - free(py_str); + value = py_str_to_c_str(py_value, NULL); + err = git_config_set_string(self->config, key, value); + free(value); } - free(c_key); + free(key); if (err < 0) { Error_set(err); return -1; @@ -263,6 +265,8 @@ Config_foreach_callback_wrapper(const git_config_entry *entry, void *c_payload) if ((c_result = PyLong_AsLong(py_result) == -1)) return -1; + Py_CLEAR(args); + return c_result; } @@ -282,7 +286,7 @@ Config_foreach(Config *self, PyObject *args) { int ret; PyObject *py_callback; - PyObject *py_payload; + PyObject *py_payload = NULL; if (!PyArg_ParseTuple(args, "O|O", &py_callback, &py_payload)) return NULL; @@ -338,13 +342,14 @@ PyDoc_STRVAR(Config_get_multivar__doc__, int Config_get_multivar_fn_wrapper(const git_config_entry *value, void *data) { - PyObject *list = (PyObject *)data; PyObject *item = NULL; if (!(item = PyUnicode_FromString(value->value))) return -2; - PyList_Append(list, item); + PyList_Append((PyObject *)data, item); + + Py_CLEAR(item); return 0; } @@ -353,17 +358,20 @@ PyObject * Config_get_multivar(Config *self, PyObject *args) { int err; - PyObject *list = PyList_New(0); + PyObject *list; const char *name = NULL; const char *regex = NULL; if (!PyArg_ParseTuple(args, "s|s", &name, ®ex)) return NULL; + list = PyList_New(0); err = git_config_get_multivar(self->config, name, regex, Config_get_multivar_fn_wrapper, (void *)list); if (err < 0) { + Py_CLEAR(list); + if (err == GIT_ENOTFOUND) Error_set(err); else @@ -454,9 +462,9 @@ PyTypeObject ConfigType = { 0, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, /* tp_flags */ + Py_TPFLAGS_DEFAULT, /* tp_flags */ Config__doc__, /* tp_doc */ - (traverseproc)Config_traverse, /* tp_traverse */ + 0, /* tp_traverse */ 0, /* tp_clear */ 0, /* tp_richcompare */ 0, /* tp_weaklistoffset */ diff --git a/src/diff.c b/src/diff.c index c9b4983..b8289d5 100644 --- a/src/diff.c +++ b/src/diff.c @@ -40,123 +40,106 @@ extern PyTypeObject IndexType; extern PyTypeObject DiffType; extern PyTypeObject HunkType; -PyTypeObject DiffEntryType; +PyTypeObject PatchType; PyObject* -diff_get_patch_byindex(git_diff_list* list, size_t i) +diff_get_patch_byindex(git_diff_list* list, size_t idx) { const git_diff_delta* delta; const git_diff_range* range; git_diff_patch* patch = NULL; - - char buffer[41]; - const char* hunk_content; - size_t hunk_amounts, j, hunk_header_len, hunk_lines; + size_t i, j, hunk_amounts, lines_in_hunk, line_len, header_len; + const char* line, *header; int err; + Hunk *py_hunk = NULL; + Patch *py_patch = NULL; - PyObject *file; - Hunk *py_hunk; - DiffEntry *py_entry = NULL; + err = git_diff_get_patch(&patch, &delta, list, idx); + if (err < 0) + return Error_set(err); - err = git_diff_get_patch(&patch, &delta, list, i); + py_patch = (Patch*) PyType_GenericNew(&PatchType, NULL, NULL); + if (py_patch != NULL) { + py_patch->old_file_path = delta->old_file.path; + py_patch->new_file_path = delta->new_file.path; + py_patch->status = delta->status; + py_patch->similarity = delta->similarity; + py_patch->old_oid = git_oid_allocfmt(&delta->old_file.oid); + py_patch->new_oid = git_oid_allocfmt(&delta->new_file.oid); - if (err == GIT_OK) { - py_entry = (DiffEntry*) INSTANCIATE_CLASS(DiffEntryType, NULL); - if (py_entry != NULL) { - if (err == GIT_OK) { - file = Py_BuildValue("(s,s,i,i)", - delta->old_file.path, - delta->new_file.path, - delta->status, - delta->similarity - ); - PyList_Append((PyObject*) py_entry->files, file); - } + hunk_amounts = git_diff_patch_num_hunks(patch); + py_patch->hunks = PyList_New(hunk_amounts); + for (i=0; i < hunk_amounts; ++i) { + err = git_diff_patch_get_hunk(&range, &header, &header_len, + &lines_in_hunk, patch, i); - hunk_amounts = git_diff_patch_num_hunks(patch); + if (err < 0) + goto cleanup; - for (j=0; j < hunk_amounts; ++j) { - err = git_diff_patch_get_hunk(&range, &hunk_content, - &hunk_header_len, &hunk_lines, patch, j); + py_hunk = (Hunk*)PyType_GenericNew(&HunkType, NULL, NULL); + if (py_hunk != NULL) { + py_hunk->old_start = range->old_start; + py_hunk->old_lines = range->old_lines; + py_hunk->new_start = range->new_start; + py_hunk->new_lines = range->new_lines; - if (err == GIT_OK) { - py_hunk = (Hunk*)PyType_GenericNew(&HunkType, NULL, NULL); - if (py_hunk != NULL) { - py_hunk->old_file = delta->old_file.path; - py_hunk->new_file = delta->new_file.path; - py_hunk->header = hunk_content; - py_hunk->old_start = range->old_start; - py_hunk->old_lines = range->old_lines; - py_hunk->new_start = range->new_start; - py_hunk->new_lines = range->new_lines; + py_hunk->lines = PyList_New(lines_in_hunk + 1); + PyList_SetItem(py_hunk->lines, 0, + PyUnicode_FromStringAndSize(header, header_len)); + for (j=1; j < lines_in_hunk + 1; ++j) { + err = git_diff_patch_get_line_in_hunk(NULL, &line, + &line_len, NULL, NULL, patch, i, j - 1); - git_oid_fmt(buffer, &delta->old_file.oid); - py_hunk->old_oid = calloc(41, sizeof(char)); - memcpy(py_hunk->old_oid, buffer, 40); + if (err < 0) + goto cleanup; - git_oid_fmt(buffer, &delta->new_file.oid); - py_hunk->new_oid = calloc(41, sizeof(char)); - memcpy(py_hunk->new_oid, buffer, 40); - - py_hunk->data = Py_BuildValue("(s#,i)", - hunk_content, hunk_header_len, - hunk_lines); - - PyList_Append((PyObject*) py_entry->hunks, - (PyObject*) py_hunk); - } + PyList_SetItem(py_hunk->lines, j, + PyUnicode_FromStringAndSize(line, line_len)); } + + PyList_SetItem((PyObject*) py_patch->hunks, i, + (PyObject*) py_hunk); } } } - if (err < 0) - return Error_set(err); +cleanup: + git_diff_patch_free(patch); - return (PyObject*) py_entry; -} - -PyObject * -DiffEntry_call(DiffEntry *self, PyObject *args, PyObject *kwds) -{ - self->files = PyList_New(0); - if (self->files == NULL) { - Py_XDECREF(self); - return NULL; - } - - self->hunks = PyList_New(0); - if (self->hunks == NULL) { - Py_XDECREF(self); - return NULL; - } - - return (PyObject*) self; + return (err < 0) ? Error_set(err) : (PyObject*) py_patch; } static void -DiffEntry_dealloc(DiffEntry *self) +Patch_dealloc(Patch *self) { - Py_DECREF((PyObject*) self->files); - Py_DECREF((PyObject*) self->hunks); + Py_CLEAR(self->hunks); + free(self->old_oid); + free(self->new_oid); + // we do not have to free old_file_path and new_file_path, they will + // be freed by git_diff_list_free in Diff_dealloc PyObject_Del(self); } -PyMemberDef DiffEntry_members[] = { - MEMBER(DiffEntry, files, T_OBJECT, "files"), - MEMBER(DiffEntry, hunks, T_OBJECT, "hunks"), +PyMemberDef Patch_members[] = { + MEMBER(Patch, old_file_path, T_STRING, "old file path"), + MEMBER(Patch, new_file_path, T_STRING, "new file path"), + MEMBER(Patch, old_oid, T_STRING, "old oid"), + MEMBER(Patch, new_oid, T_STRING, "new oid"), + MEMBER(Patch, status, T_INT, "status"), + MEMBER(Patch, similarity, T_INT, "similarity"), + MEMBER(Patch, hunks, T_OBJECT, "hunks"), {NULL} }; -PyDoc_STRVAR(DiffEntry__doc__, "Diff entry object."); +PyDoc_STRVAR(Patch__doc__, "Diff patch object."); -PyTypeObject DiffEntryType = { +PyTypeObject PatchType = { PyVarObject_HEAD_INIT(NULL, 0) - "_pygit2.DiffEntry", /* tp_name */ - sizeof(DiffEntry), /* tp_basicsize */ + "_pygit2.Patch", /* tp_name */ + sizeof(Patch), /* tp_basicsize */ 0, /* tp_itemsize */ - (destructor)DiffEntry_dealloc, /* tp_dealloc */ + (destructor)Patch_dealloc, /* tp_dealloc */ 0, /* tp_print */ 0, /* tp_getattr */ 0, /* tp_setattr */ @@ -166,13 +149,13 @@ PyTypeObject DiffEntryType = { 0, /* tp_as_sequence */ 0, /* tp_as_mapping */ 0, /* tp_hash */ - (ternaryfunc) DiffEntry_call, /* tp_call */ + 0, /* tp_call */ 0, /* tp_str */ 0, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ - DiffEntry__doc__, /* tp_doc */ + Patch__doc__, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ 0, /* tp_richcompare */ @@ -180,7 +163,7 @@ PyTypeObject DiffEntryType = { 0, /* tp_iter */ 0, /* tp_iternext */ 0, /* tp_methods */ - DiffEntry_members, /* tp_members */ + Patch_members, /* tp_members */ 0, /* tp_getset */ 0, /* tp_base */ 0, /* tp_dict */ @@ -197,7 +180,7 @@ PyObject * DiffIter_iternext(DiffIter *self) { if (self->i < self->n) - return diff_get_patch_byindex(self->list, self->i++); + return diff_get_patch_byindex(self->diff->list, self->i++); PyErr_SetNone(PyExc_StopIteration); return NULL; @@ -206,7 +189,7 @@ DiffIter_iternext(DiffIter *self) void DiffIter_dealloc(DiffIter *self) { - Py_CLEAR(self->list); + Py_CLEAR(self->diff); PyObject_Del(self); } @@ -251,37 +234,39 @@ Diff_patch__get__(Diff *self) { const git_diff_delta* delta; git_diff_patch* patch; - char* str = NULL, *buffer = NULL; - int err = 0; - size_t i, len, num, size; + char **strings = NULL; + char *buffer = NULL; + int err = GIT_ERROR; + size_t i, len, num; PyObject *py_patch = NULL; num = git_diff_num_deltas(self->list); - for (i = 0; i < num ; ++i) { + MALLOC(strings, num * sizeof(char*), cleanup); + + for (i = 0, len = 1; i < num ; ++i) { err = git_diff_get_patch(&patch, &delta, self->list, i); + if (err < 0) + goto cleanup; + + err = git_diff_patch_to_str(&(strings[i]), patch); + if (err < 0) + goto cleanup; - if (err < 0 || (err = git_diff_patch_to_str(&str, patch)) < 0) - goto error; - - len = strlen(str) + 1; - size = (buffer == NULL) ? len : strlen(buffer) + len; - MALLOC(buffer, size, error); - - if (len == size) - strcpy(buffer, str); - else - strcat(buffer, str); - - FREE(str); + len += strlen(strings[i]); + git_diff_patch_free(patch); } + CALLOC(buffer, (len + 1), sizeof(char), cleanup); + for (i = 0; i < num; ++i) { + strcat(buffer, strings[i]); + free(strings[i]); + } + free(strings); + py_patch = PyUnicode_FromString(buffer); + free(buffer); -error: - FREE(str); - FREE(buffer); - FREE_FUNC(patch, git_diff_patch_free); - +cleanup: return (err < 0) ? Error_set(err) : py_patch; } @@ -289,34 +274,16 @@ error: static void Hunk_dealloc(Hunk *self) { - if (self->header != NULL) { - free((void*) self->header); - } - if (self->new_file != NULL) { - free((void*) self->new_file); - } - if (self->old_file != NULL) { - free((void*) self->old_file); - } - Py_XDECREF(self->old_oid); - Py_XDECREF(self->new_oid); - Py_XDECREF(self->data); + Py_CLEAR(self->lines); PyObject_Del(self); } PyMemberDef Hunk_members[] = { - MEMBER(Hunk, header, T_STRING, "Header."), MEMBER(Hunk, old_start, T_INT, "Old start."), MEMBER(Hunk, old_lines, T_INT, "Old lines."), - MEMBER(Hunk, old_mode, T_INT, "Old mode."), - MEMBER(Hunk, old_file, T_STRING, "Old file."), - MEMBER(Hunk, old_oid, T_STRING, "Old oid."), MEMBER(Hunk, new_start, T_INT, "New start."), MEMBER(Hunk, new_lines, T_INT, "New lines."), - MEMBER(Hunk, new_mode, T_INT, "New mode."), - MEMBER(Hunk, new_file, T_STRING, "New file."), - MEMBER(Hunk, new_oid, T_STRING, "New oid."), - MEMBER(Hunk, data, T_OBJECT, "Data."), + MEMBER(Hunk, lines, T_OBJECT, "Lines."), {NULL} }; @@ -393,7 +360,7 @@ Diff_merge(Diff *self, PyObject *args) PyDoc_STRVAR(Diff_find_similar__doc__, "find_similar([flags])\n" "\n" - "Find renamed files in diff."); + "Find renamed files in diff and updates them in-place in the diff itself."); PyObject * Diff_find_similar(Diff *self, PyObject *args) @@ -417,9 +384,9 @@ Diff_iter(Diff *self) DiffIter *iter; iter = PyObject_New(DiffIter, &DiffIterType); - if (iter) { + if (iter != NULL) { Py_INCREF(self); - iter->list = self->list; + iter->diff = self; iter->i = 0; iter->n = git_diff_num_deltas(self->list); } @@ -444,7 +411,7 @@ static void Diff_dealloc(Diff *self) { git_diff_list_free(self->list); - Py_XDECREF(self->repo); + Py_CLEAR(self->repo); PyObject_Del(self); } diff --git a/src/object.c b/src/object.c index 40e30e4..314bac2 100644 --- a/src/object.c +++ b/src/object.c @@ -43,8 +43,8 @@ extern PyTypeObject TagType; void Object_dealloc(Object* self) { + Py_CLEAR(self->repo); git_object_free(self->obj); - Py_XDECREF(self->repo); PyObject_Del(self); } diff --git a/src/pygit2.c b/src/pygit2.c index e713c0c..09573ef 100644 --- a/src/pygit2.c +++ b/src/pygit2.c @@ -42,7 +42,7 @@ extern PyTypeObject ObjectType; extern PyTypeObject CommitType; extern PyTypeObject DiffType; extern PyTypeObject DiffIterType; -extern PyTypeObject DiffEntryType; +extern PyTypeObject PatchType; extern PyTypeObject HunkType; extern PyTypeObject TreeType; extern PyTypeObject TreeBuilderType; @@ -201,7 +201,7 @@ moduleinit(PyObject* m) return NULL; if (PyType_Ready(&DiffIterType) < 0) return NULL; - if (PyType_Ready(&DiffEntryType) < 0) + if (PyType_Ready(&PatchType) < 0) return NULL; if (PyType_Ready(&HunkType) < 0) return NULL; diff --git a/src/reference.c b/src/reference.c index 129ed44..b508567 100644 --- a/src/reference.c +++ b/src/reference.c @@ -43,9 +43,8 @@ extern PyTypeObject RefLogEntryType; void RefLogIter_dealloc(RefLogIter *self) { - Py_XDECREF(self->reference); git_reflog_free(self->reflog); - PyObject_GC_Del(self); + PyObject_Del(self); } PyObject* RefLogIter_iternext(PyObject *self) @@ -146,7 +145,9 @@ Reference_delete(Reference *self, PyObject *args) if (err < 0) return Error_set(err); + git_reference_free(self->reference); self->reference = NULL; /* Invalidate the pointer */ + Py_RETURN_NONE; } @@ -172,6 +173,7 @@ Reference_rename(Reference *self, PyObject *py_name) /* Rename */ err = git_reference_rename(&new_reference, self->reference, c_name, 0); + git_reference_free(self->reference); free(c_name); if (err < 0) return Error_set(err); @@ -255,6 +257,7 @@ Reference_target__set__(Reference *self, PyObject *py_name) return -1; } + git_reference_free(self->reference); self->reference = new_ref; return 0; } @@ -316,6 +319,7 @@ Reference_oid__set__(Reference *self, PyObject *py_hex) return -1; } + git_reference_free(self->reference); self->reference = new_ref; return 0; } @@ -371,14 +375,10 @@ Reference_log(Reference *self) CHECK_REFERENCE(self); iter = PyObject_New(RefLogIter, &RefLogIterType); - if (iter) { - iter->reference = self; + if (iter != NULL) { git_reflog_read(&iter->reflog, self->reference); iter->size = git_reflog_entrycount(iter->reflog); iter->i = 0; - - Py_INCREF(self); - Py_INCREF(iter); } return (PyObject*)iter; } @@ -398,9 +398,9 @@ RefLogEntry_init(RefLogEntry *self, PyObject *args, PyObject *kwds) static void RefLogEntry_dealloc(RefLogEntry *self) { - Py_XDECREF(self->oid_old); - Py_XDECREF(self->oid_new); - Py_XDECREF(self->committer); + Py_CLEAR(self->oid_old); + Py_CLEAR(self->oid_new); + Py_CLEAR(self->committer); free(self->message); PyObject_Del(self); } diff --git a/src/remote.c b/src/remote.c index ccf7e55..0d55a54 100644 --- a/src/remote.c +++ b/src/remote.c @@ -47,6 +47,7 @@ Remote_init(Remote *self, PyObject *args, PyObject *kwds) return NULL; self->repo = py_repo; + Py_INCREF(self->repo); err = git_remote_load(&self->remote, py_repo->repo, name); if (err < 0) @@ -59,6 +60,7 @@ Remote_init(Remote *self, PyObject *args, PyObject *kwds) static void Remote_dealloc(Remote *self) { + Py_CLEAR(self->repo); git_remote_free(self->remote); PyObject_Del(self); } @@ -81,6 +83,7 @@ Remote_name__set__(Remote *self, PyObject* py_name) name = py_str_to_c_str(py_name, NULL); if (name != NULL) { err = git_remote_rename(self->remote, name, NULL, NULL); + free(name); if (err == GIT_OK) return 0; @@ -105,11 +108,12 @@ int Remote_url__set__(Remote *self, PyObject* py_url) { int err; - char* url; + char* url = NULL; url = py_str_to_c_str(py_url, NULL); if (url != NULL) { err = git_remote_set_url(self->remote, url); + free(url); if (err == GIT_OK) return 0; diff --git a/src/repository.c b/src/repository.c index bba47e3..87ff395 100644 --- a/src/repository.c +++ b/src/repository.c @@ -102,6 +102,9 @@ Repository_init(Repository *self, PyObject *args, PyObject *kwds) return -1; } + self->config = NULL; + self->index = NULL; + return 0; } @@ -109,7 +112,8 @@ void Repository_dealloc(Repository *self) { PyObject_GC_UnTrack(self); - Py_XDECREF(self->index); + Py_CLEAR(self->index); + Py_CLEAR(self->config); git_repository_free(self->repo); PyObject_GC_Del(self); } @@ -494,7 +498,7 @@ PyDoc_STRVAR(Repository_config__doc__, "(if they are available)."); PyObject * -Repository_config__get__(Repository *self, void *closure) +Repository_config__get__(Repository *self) { int err; git_config *config; @@ -507,20 +511,18 @@ Repository_config__get__(Repository *self, void *closure) if (err < 0) return Error_set(err); - py_config = PyObject_GC_New(Config, &ConfigType); - if (!py_config) { + py_config = PyObject_New(Config, &ConfigType); + if (py_config == NULL) { git_config_free(config); return NULL; } - Py_INCREF(self); - py_config->repo = self; py_config->config = config; - PyObject_GC_Track(py_config); self->config = (PyObject*)py_config; + } else { + Py_INCREF(self->config); } - Py_INCREF(self->config); return self->config; } @@ -815,6 +817,7 @@ Repository_lookup_reference(Repository *self, PyObject *py_name) return err_obj; } free(c_name); + /* 3- Make an instance of Reference and return it */ return wrap_reference(c_reference); } @@ -888,13 +891,19 @@ Repository_create_symbolic_reference(Repository *self, PyObject *args, #if PY_MAJOR_VERSION == 2 c_target = PyString_AsString(py_obj); #else - c_target = PyString_AsString(PyUnicode_AsASCIIString(py_obj)); + // increases ref counter, so we have to release it afterwards + PyObject* py_str = PyUnicode_AsASCIIString(py_obj); + c_target = PyString_AsString(py_str); #endif if (c_target == NULL) return NULL; err = git_reference_symbolic_create(&c_reference, self->repo, c_name, c_target, force); + #if PY_MAJOR_VERSION > 2 + Py_CLEAR(py_str); + #endif + if (err < 0) return Error_set(err); @@ -914,9 +923,14 @@ read_status_cb(const char *path, unsigned int status_flags, void *payload) /* This is the callback that will be called in git_status_foreach. It * will be called for every path.*/ PyObject *flags; + int err; flags = PyInt_FromLong((long) status_flags); - PyDict_SetItemString(payload, path, flags); + err = PyDict_SetItemString(payload, path, flags); + Py_CLEAR(flags); + + if (err < 0) + return GIT_ERROR; return GIT_OK; } @@ -1103,6 +1117,7 @@ Repository_checkout(Repository *self, PyObject *args, PyObject *kw) err = git_repository_set_head(self->repo, git_reference_name(ref->reference)); } + git_object_free(object); } } else { /* checkout from head / index */ opts.checkout_strategy = strategy; diff --git a/src/signature.c b/src/signature.c index b0e8566..25dae0b 100644 --- a/src/signature.c +++ b/src/signature.c @@ -82,12 +82,13 @@ void Signature_dealloc(Signature *self) { if (self->obj) - Py_DECREF(self->obj); + Py_CLEAR(self->obj); else { git_signature_free((git_signature*)self->signature); free((char*)self->encoding); } - Py_TYPE(self)->tp_free((PyObject*)self); + + PyObject_Del(self); } @@ -221,6 +222,7 @@ build_signature(Object *obj, const git_signature *signature, Signature *py_signature; py_signature = PyObject_New(Signature, &SignatureType); + if (py_signature) { Py_INCREF(obj); py_signature->obj = obj; diff --git a/src/tree.c b/src/tree.c index 7493299..6121c3f 100644 --- a/src/tree.c +++ b/src/tree.c @@ -42,7 +42,7 @@ extern PyTypeObject IndexType; void TreeEntry_dealloc(TreeEntry *self) { - Py_XDECREF(self->owner); + Py_CLEAR(self->owner); git_tree_entry_free((git_tree_entry*)self->entry); PyObject_Del(self); } @@ -339,6 +339,7 @@ Tree_diff(Tree *self, PyObject *args) PyErr_SetObject(PyExc_TypeError, py_obj); return NULL; } + if (err < 0) return Error_set(err); @@ -352,6 +353,7 @@ Tree_diff(Tree *self, PyObject *args) return (PyObject*)py_diff; } + PySequenceMethods Tree_as_sequence = { 0, /* sq_length */ 0, /* sq_concat */ diff --git a/src/treebuilder.c b/src/treebuilder.c index 1b06b31..d7205bc 100644 --- a/src/treebuilder.c +++ b/src/treebuilder.c @@ -37,7 +37,7 @@ void TreeBuilder_dealloc(TreeBuilder *self) { - Py_XDECREF(self->repo); + Py_CLEAR(self->repo); git_treebuilder_free(self->bld); PyObject_Del(self); } diff --git a/src/utils.c b/src/utils.c index cfa9cae..efd7887 100644 --- a/src/utils.c +++ b/src/utils.c @@ -36,13 +36,13 @@ extern PyTypeObject ReferenceType; * the string contained in the value argument. */ char * py_str_to_c_str(PyObject *value, const char *encoding) { + char *c_str = NULL; /* Case 1: byte string */ if (PyString_Check(value)) return strdup(PyString_AsString(value)); /* Case 2: text string */ if (PyUnicode_Check(value)) { - char *c_str = NULL; if (encoding == NULL) value = PyUnicode_AsUTF8String(value); diff --git a/src/walker.c b/src/walker.c index d2481a1..7e48ab3 100644 --- a/src/walker.c +++ b/src/walker.c @@ -38,8 +38,8 @@ extern PyTypeObject CommitType; void Walker_dealloc(Walker *self) { + Py_CLEAR(self->repo); git_revwalk_free(self->walk); - Py_DECREF(self->repo); PyObject_Del(self); } diff --git a/test/test_diff.py b/test/test_diff.py index 8cb7b2c..54290fc 100644 --- a/test/test_diff.py +++ b/test/test_diff.py @@ -31,7 +31,6 @@ from __future__ import absolute_import from __future__ import unicode_literals import unittest import pygit2 -import itertools from pygit2 import GIT_DIFF_INCLUDE_UNMODIFIED from . import utils @@ -84,22 +83,27 @@ DIFF_WORKDIR_EXPECTED = [ 'subdir/modified_file' ] +HUNK_EXPECTED = """@@ -1 +1 @@ +a contents 2 +a contents +""" + class DiffDirtyTest(utils.DirtyRepoTestCase): def test_diff_empty_index(self): repo = self.repo head = repo[repo.lookup_reference('HEAD').resolve().oid] diff = head.tree.diff(repo.index) - files = [[x[0] for x in entry.files] for entry in diff] - self.assertEqual(DIFF_INDEX_EXPECTED, list(itertools.chain(*files))) + files = [patch.new_file_path for patch in diff] + self.assertEqual(DIFF_INDEX_EXPECTED, files) def test_workdir_to_tree(self): repo = self.repo head = repo[repo.lookup_reference('HEAD').resolve().oid] diff = head.tree.diff() - files = [[x[0] for x in entry.files] for entry in diff] - self.assertEqual(DIFF_WORKDIR_EXPECTED, list(itertools.chain(*files))) + files = [patch.new_file_path for patch in diff] + self.assertEqual(DIFF_WORKDIR_EXPECTED, files) class DiffTest(utils.BareRepoTestCase): @@ -113,8 +117,8 @@ class DiffTest(utils.BareRepoTestCase): head = repo[repo.lookup_reference('HEAD').resolve().oid] diff = head.tree.diff(repo.index) - files = [[x[0].split('/')[0] for x in entry.files] for entry in diff] - self.assertEqual([x.name for x in head.tree], list(itertools.chain(*files))) + files = [patch.new_file_path.split('/')[0] for patch in diff] + self.assertEqual([x.name for x in head.tree], files) def test_diff_tree(self): commit_a = self.repo[COMMIT_SHA1_1] @@ -125,20 +129,17 @@ class DiffTest(utils.BareRepoTestCase): # self.assertIsNotNone is 2.7 only self.assertTrue(diff is not None) # self.assertIn is 2.7 only - self.assertAny(lambda x: ('a', 'a', 3, 0) in x.files, diff) self.assertEqual(2, sum(map(lambda x: len(x.hunks), diff))) - hunk = diff[0].hunks[0] + patch = diff[0] + hunk = patch.hunks[0] self.assertEqual(hunk.old_start, 1) self.assertEqual(hunk.old_lines, 1) self.assertEqual(hunk.new_start, 1) self.assertEqual(hunk.new_lines, 1) - self.assertEqual(hunk.old_file, 'a') - self.assertEqual(hunk.new_file, 'a') - - #self.assertEqual(hunk.data[0][0], b'a contents 2\n') - #self.assertEqual(hunk.data[1][0], b'a contents\n') + self.assertEqual(patch.old_file_path, 'a') + self.assertEqual(patch.new_file_path, 'a') def test_diff_tree_opts(self): commit_c = self.repo[COMMIT_SHA1_3] @@ -168,25 +169,23 @@ class DiffTest(utils.BareRepoTestCase): self.assertTrue(diff_c is not None) # assertIn / assertNotIn are 2.7 only - self.assertAll(lambda x:('b', 'b', 3, 0) not in x.files, diff_b) - self.assertAny(lambda x:('b', 'b', 3, 0) in x.files, diff_c) + self.assertFalse('b' in [patch.new_file_path for patch in diff_b]) + self.assertTrue('b' in [patch.new_file_path for patch in diff_c]) diff_b.merge(diff_c) # assertIn is 2.7 only - self.assertAny(lambda x:('b', 'b', 3, 0) in x.files, diff_b) + self.assertTrue('b' in [patch.new_file_path for patch in diff_b]) - hunk = diff_b[1].hunks[0] + patch = diff_b[0] + hunk = patch.hunks[0] self.assertEqual(hunk.old_start, 1) self.assertEqual(hunk.old_lines, 1) self.assertEqual(hunk.new_start, 1) self.assertEqual(hunk.new_lines, 1) - self.assertEqual(hunk.old_file, 'b') - self.assertEqual(hunk.new_file, 'b') - - #self.assertEqual(hunk.data[0][0], b'b contents\n') - #self.assertEqual(hunk.data[1][0], b'b contents 2\n') + self.assertEqual(patch.old_file_path, 'a') + self.assertEqual(patch.new_file_path, 'a') def test_diff_patch(self): commit_a = self.repo[COMMIT_SHA1_1] @@ -195,23 +194,22 @@ class DiffTest(utils.BareRepoTestCase): diff = commit_a.tree.diff(commit_b.tree) self.assertEqual(diff.patch, PATCH) - def test_diff_header(self): - commit_a = self.repo[COMMIT_SHA1_1] - commit_b = self.repo[COMMIT_SHA1_2] - diff = commit_a.tree.diff(commit_b.tree) - - self.assertEqual(diff[0].hunks[0].header, "@@ -1 +1 @@\n") - def test_diff_oids(self): commit_a = self.repo[COMMIT_SHA1_1] commit_b = self.repo[COMMIT_SHA1_2] - diff = commit_a.tree.diff(commit_b.tree) - hunk = diff[0].hunks[0] - self.assertEqual(hunk.old_oid, + patch = commit_a.tree.diff(commit_b.tree)[0] + self.assertEqual(patch.old_oid, '7f129fd57e31e935c6d60a0c794efe4e6927664b') - self.assertEqual(hunk.new_oid, + self.assertEqual(patch.new_oid, 'af431f20fc541ed6d5afede3e2dc7160f6f01f16') + def test_hunk_content(self): + commit_a = self.repo[COMMIT_SHA1_1] + commit_b = self.repo[COMMIT_SHA1_2] + patch = commit_a.tree.diff(commit_b.tree)[0] + hunk = patch.hunks[0] + self.assertEqual(HUNK_EXPECTED, ''.join(hunk.lines)) + def test_find_similar(self): commit_a = self.repo[COMMIT_SHA1_6] commit_b = self.repo[COMMIT_SHA1_7] @@ -219,10 +217,9 @@ class DiffTest(utils.BareRepoTestCase): #~ Must pass GIT_DIFF_INCLUDE_UNMODIFIED if you expect to emulate #~ --find-copies-harder during rename transformion... diff = commit_a.tree.diff(commit_b.tree, GIT_DIFF_INCLUDE_UNMODIFIED) - entry = ('lorem', 'ipsum', pygit2.GIT_DELTA_RENAMED, 100) - self.assertAll(lambda x: entry not in x.files, diff) + self.assertAll(lambda x: x.status is not pygit2.GIT_DELTA_RENAMED, diff) diff.find_similar() - self.assertAny(lambda x: entry in x.files, diff) + self.assertAny(lambda x: x.status is pygit2.GIT_DELTA_RENAMED, diff) if __name__ == '__main__': unittest.main()