From 74d091d6097f7792bac72dbf8e3da22560714b8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 3 Feb 2014 17:45:49 +0100 Subject: [PATCH 1/7] utils: allow borrowing a C string We don't always need our own copy of a C string; sometimes we just need to parse it. Create py_str_borrow_c_str() which returns a char pointer to python's internal value string, with which we can avoid an extra copy. --- src/utils.c | 43 +++++++++++++++++++++++++++++++++---------- src/utils.h | 2 ++ 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/src/utils.c b/src/utils.c index 6a20b01bf..e63ea9dbf 100644 --- a/src/utils.c +++ b/src/utils.c @@ -32,27 +32,50 @@ extern PyTypeObject ReferenceType; -/* py_str_to_c_str() returns a newly allocated C string holding - * the string contained in the value argument. */ +/** + * py_str_to_c_str() returns a newly allocated C string holding the string + * contained in the 'value' argument. + */ char * py_str_to_c_str(PyObject *value, const char *encoding) { + const char *borrowed; char *c_str = NULL; + PyObject *tmp = NULL; + + borrowed = py_str_borrow_c_str(&tmp, value, encoding); + if (!borrowed) + return NULL; + + c_str = strdup(borrowed); + + Py_DECREF(tmp); + return c_str; +} + +/** + * Return a pointer to the underlying C string in 'value'. The pointer is + * guaranteed by 'tvalue'. Decrease its refcount when done with the string. + */ +const char * +py_str_borrow_c_str(PyObject **tvalue, PyObject *value, const char *encoding) +{ /* Case 1: byte string */ - if (PyBytes_Check(value)) - return strdup(PyBytes_AsString(value)); + if (PyBytes_Check(value)) { + Py_INCREF(value); + *tvalue = value; + return PyBytes_AsString(value); + } /* Case 2: text string */ if (PyUnicode_Check(value)) { if (encoding == NULL) - value = PyUnicode_AsUTF8String(value); + *tvalue = PyUnicode_AsUTF8String(value); else - value = PyUnicode_AsEncodedString(value, encoding, "strict"); - if (value == NULL) + *tvalue = PyUnicode_AsEncodedString(value, encoding, "strict"); + if (*tvalue == NULL) return NULL; - c_str = strdup(PyBytes_AsString(value)); - Py_DECREF(value); - return c_str; + return PyBytes_AsString(*tvalue); } /* Type error */ diff --git a/src/utils.h b/src/utils.h index 05ddcd21b..3b3c42a07 100644 --- a/src/utils.h +++ b/src/utils.h @@ -108,6 +108,8 @@ to_bytes(const char * value) } char * py_str_to_c_str(PyObject *value, const char *encoding); +const char *py_str_borrow_c_str(PyObject **tvaue, PyObject *value, const char *encoding); + #define py_path_to_c_str(py_path) \ py_str_to_c_str(py_path, Py_FileSystemDefaultEncoding) From 1c74676ed405a5ccd199080618417158589f39c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 3 Feb 2014 17:56:59 +0100 Subject: [PATCH 2/7] config: borrow the string for lookup and setting We don't need our own copy of the string, so use the new borrowing mechanism to use python's underlying string for the key to get/set. --- src/config.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/config.c b/src/config.c index 441abb127..8cbb05451 100644 --- a/src/config.c +++ b/src/config.c @@ -147,15 +147,15 @@ Config_get_system_config(void) int Config_contains(Config *self, PyObject *py_key) { int err; - const char *c_value; - char *c_key; + const char *c_value, *c_key; + PyObject *tkey; - c_key = py_str_to_c_str(py_key, NULL); + c_key = py_str_borrow_c_str(&tkey, py_key, NULL); if (c_key == NULL) return -1; err = git_config_get_string(&c_value, self->config, c_key); - free(c_key); + Py_DECREF(tkey); if (err < 0) { if (err == GIT_ENOTFOUND) @@ -175,14 +175,15 @@ Config_getitem(Config *self, PyObject *py_key) int64_t value_int; int err, value_bool; const char *value_str; - char *key; - PyObject* py_value; + const char *key; + PyObject* py_value, *tmp; - key = py_str_to_c_str(py_key, NULL); + key = py_str_borrow_c_str(&tmp, py_key, NULL); if (key == NULL) return NULL; err = git_config_get_string(&value_str, self->config, key); + Py_CLEAR(tmp); if (err < 0) goto cleanup; @@ -194,8 +195,6 @@ Config_getitem(Config *self, PyObject *py_key) py_value = to_unicode(value_str, NULL, NULL); cleanup: - free(key); - if (err < 0) { if (err == GIT_ENOTFOUND) { PyErr_SetObject(PyExc_KeyError, py_key); @@ -212,9 +211,10 @@ int Config_setitem(Config *self, PyObject *py_key, PyObject *py_value) { int err; - char *key, *value; + const char *key, *value; + PyObject *tkey, *tvalue; - key = py_str_to_c_str(py_key, NULL); + key = py_str_borrow_c_str(&tkey, py_key, NULL); if (key == NULL) return -1; @@ -227,12 +227,12 @@ Config_setitem(Config *self, PyObject *py_key, PyObject *py_value) err = git_config_set_int64(self->config, key, (int64_t)PyLong_AsLong(py_value)); } else { - value = py_str_to_c_str(py_value, NULL); + value = py_str_borrow_c_str(&tvalue, py_value, NULL); err = git_config_set_string(self->config, key, value); - free(value); + Py_DECREF(tvalue); } - free(key); + Py_DECREF(tkey); if (err < 0) { Error_set(err); return -1; From b4827ba0815092b5008a4b4b211bfc5e5da63a1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 4 Feb 2014 12:32:08 +0100 Subject: [PATCH 3/7] repository: borrow C strings where possible --- src/repository.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/repository.c b/src/repository.c index 95e2e3459..750517859 100644 --- a/src/repository.c +++ b/src/repository.c @@ -181,14 +181,15 @@ int Repository_head__set__(Repository *self, PyObject *py_refname) { int err; - char *refname; + const char *refname; + PyObject *trefname; - refname = py_str_to_c_str(py_refname, NULL); + refname = py_str_borrow_c_str(&trefname, py_refname, NULL); if (refname == NULL) return -1; err = git_repository_set_head(self->repo, refname); - free(refname); + Py_DECREF(trefname); if (err < 0) { Error_set_str(err, refname); return -1; @@ -318,11 +319,12 @@ PyObject * Repository_revparse_single(Repository *self, PyObject *py_spec) { git_object *c_obj; - char *c_spec; + const char *c_spec; + PyObject *tspec; int err; /* 1- Get the C revision spec */ - c_spec = py_str_to_c_str(py_spec, NULL); + c_spec = py_str_borrow_c_str(&tspec, py_spec, NULL); if (c_spec == NULL) return NULL; @@ -331,10 +333,10 @@ Repository_revparse_single(Repository *self, PyObject *py_spec) if (err < 0) { PyObject *err_obj = Error_set_str(err, c_spec); - free(c_spec); + Py_DECREF(tspec); return err_obj; } - free(c_spec); + Py_DECREF(tspec); return wrap_object(c_obj, self); } @@ -782,7 +784,8 @@ Repository_create_commit(Repository *self, PyObject *args) Signature *py_author, *py_committer; PyObject *py_oid, *py_message, *py_parents, *py_parent; PyObject *py_result = NULL; - char *message = NULL; + PyObject *tmessage; + const char *message = NULL; char *update_ref = NULL; char *encoding = NULL; git_oid oid; @@ -806,7 +809,7 @@ Repository_create_commit(Repository *self, PyObject *args) if (len == 0) goto out; - message = py_str_to_c_str(py_message, encoding); + message = py_str_borrow_c_str(&tmessage, py_message, encoding); if (message == NULL) goto out; @@ -846,7 +849,7 @@ Repository_create_commit(Repository *self, PyObject *args) py_result = git_oid_to_python(&oid); out: - free(message); + Py_DECREF(tmessage); git_tree_free(tree); while (i > 0) { i--; @@ -1048,7 +1051,7 @@ Repository_lookup_reference(Repository *self, PyObject *py_name) err = git_reference_lookup(&c_reference, self->repo, c_name); if (err < 0) { PyObject *err_obj = Error_set_str(err, c_name); - free(c_name); + free(c_name); return err_obj; } free(c_name); From c8a4027affef36dc9caf4398961991e3c5f4bf75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 4 Feb 2014 12:35:07 +0100 Subject: [PATCH 4/7] signature: borrow the name's C string --- src/signature.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/signature.c b/src/signature.c index 687343b89..809e80285 100644 --- a/src/signature.c +++ b/src/signature.c @@ -37,8 +37,9 @@ int Signature_init(Signature *self, PyObject *args, PyObject *kwds) { char *keywords[] = {"name", "email", "time", "offset", "encoding", NULL}; - PyObject *py_name; - char *name, *email, *encoding = "ascii"; + PyObject *py_name, *tname; + char *email, *encoding = "ascii"; + const char *name; long long time = -1; int offset = 0; int err; @@ -49,7 +50,7 @@ Signature_init(Signature *self, PyObject *args, PyObject *kwds) &py_name, &email, &time, &offset, &encoding)) return -1; - name = py_str_to_c_str(py_name, encoding); + name = py_str_borrow_c_str(&tname, py_name, encoding); if (name == NULL) return -1; @@ -58,7 +59,8 @@ Signature_init(Signature *self, PyObject *args, PyObject *kwds) } else { err = git_signature_new(&signature, name, email, time, offset); } - free(name); + Py_DECREF(tname); + if (err < 0) { Error_set(err); return -1; From 659749510f17cc5cce03b3c585c687a131c880d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 4 Feb 2014 12:41:46 +0100 Subject: [PATCH 5/7] refspec: borrow the C string --- src/refspec.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/refspec.c b/src/refspec.c index a59350afd..3053fe60b 100644 --- a/src/refspec.c +++ b/src/refspec.c @@ -105,15 +105,16 @@ PyDoc_STRVAR(Refspec_src_matches__doc__, PyObject * Refspec_src_matches(Refspec *self, PyObject *py_str) { - char *str; + const char *str; + PyObject *tstr; int res; - str = py_str_to_c_str(py_str, NULL); + str = py_str_borrow_c_str(&tstr, py_str, NULL); if (!str) return NULL; res = git_refspec_src_matches(self->refspec, str); - free(str); + Py_DECREF(tstr); if (res) Py_RETURN_TRUE; @@ -129,15 +130,16 @@ PyDoc_STRVAR(Refspec_dst_matches__doc__, PyObject * Refspec_dst_matches(Refspec *self, PyObject *py_str) { - char *str; + const char *str; + PyObject *tstr; int res; - str = py_str_to_c_str(py_str, NULL); + str = py_str_borrow_c_str(&tstr, py_str, NULL); if (!str) return NULL; res = git_refspec_dst_matches(self->refspec, str); - free(str); + Py_DECREF(tstr); if (res) Py_RETURN_TRUE; @@ -153,24 +155,25 @@ PyDoc_STRVAR(Refspec_transform__doc__, PyObject * Refspec_transform(Refspec *self, PyObject *py_str) { - char *str, *trans; + const char *str; + char *trans; int err, len, alen; - PyObject *py_trans; + PyObject *py_trans, *tstr; - str = py_str_to_c_str(py_str, NULL); + str = py_str_borrow_c_str(&tstr, py_str, NULL); alen = len = strlen(str); do { alen *= alen; trans = malloc(alen); if (!trans) { - free(str); + Py_DECREF(tstr); return PyErr_NoMemory(); } err = git_refspec_transform(trans, alen, self->refspec, str); } while(err == GIT_EBUFS); - free(str); + Py_DECREF(tstr); if (err < 0) { free(trans); @@ -193,24 +196,25 @@ PyDoc_STRVAR(Refspec_rtransform__doc__, PyObject * Refspec_rtransform(Refspec *self, PyObject *py_str) { - char *str, *trans; + const char *str; + char *trans; int err, len, alen; - PyObject *py_trans; + PyObject *py_trans, *tstr; - str = py_str_to_c_str(py_str, NULL); + str = py_str_borrow_c_str(&tstr, py_str, NULL); alen = len = strlen(str); do { alen *= alen; trans = malloc(alen); if (!trans) { - free(str); + Py_DECREF(tstr); return PyErr_NoMemory(); } err = git_refspec_rtransform(trans, alen, self->refspec, str); } while(err == GIT_EBUFS); - free(str); + Py_DECREF(tstr); if (err < 0) { free(trans); From 824ac672c19c4de83a0fe41ea82714a0ef03ab6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 4 Feb 2014 12:43:54 +0100 Subject: [PATCH 6/7] remote: borrow the C string where possible --- src/remote.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/remote.c b/src/remote.c index f6ff1e3e5..89c5dda55 100644 --- a/src/remote.c +++ b/src/remote.c @@ -226,12 +226,13 @@ int Remote_name__set__(Remote *self, PyObject* py_name) { int err; - char* name; + const char* name; + PyObject *tname; - name = py_str_to_c_str(py_name, NULL); + name = py_str_borrow_c_str(&tname, py_name, NULL); if (name != NULL) { err = git_remote_rename(self->remote, name, NULL, NULL); - free(name); + Py_DECREF(tname); if (err == GIT_OK) return 0; @@ -404,12 +405,13 @@ int Remote_url__set__(Remote *self, PyObject* py_url) { int err; - char* url = NULL; + const char* url = NULL; + PyObject *turl; - url = py_str_to_c_str(py_url, NULL); + url = py_str_borrow_c_str(&turl, py_url, NULL); if (url != NULL) { err = git_remote_set_url(self->remote, url); - free(url); + Py_DECREF(turl); if (err == GIT_OK) return 0; @@ -440,12 +442,13 @@ int Remote_push_url__set__(Remote *self, PyObject* py_url) { int err; - char* url = NULL; + const char* url = NULL; + PyObject *turl; - url = py_str_to_c_str(py_url, NULL); + url = py_str_borrow_c_str(&turl, py_url, NULL); if (url != NULL) { err = git_remote_set_pushurl(self->remote, url); - free(url); + Py_DECREF(turl); if (err == GIT_OK) return 0; From dcd5acc34eacb2024a2b9e7e206717119eea0240 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 4 Feb 2014 12:45:50 +0100 Subject: [PATCH 7/7] index entry: avoid extra copy --- src/index.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/index.c b/src/index.c index 5d70a1dcf..90fae7628 100644 --- a/src/index.c +++ b/src/index.c @@ -650,18 +650,12 @@ IndexEntry_path__get__(IndexEntry *self) int IndexEntry_path__set__(IndexEntry *self, PyObject *py_path) { - char *c_inner, *c_path; + char *c_path; - c_inner = py_str_to_c_str(py_path, NULL); - if (!c_inner) + c_path = py_str_to_c_str(py_path, NULL); + if (!c_path) return -1; - c_path = strdup(c_inner); - if (!c_path) { - PyErr_NoMemory(); - return -1; - } - free(self->entry.path); self->entry.path = c_path;