Safer repository pointer extraction

Casting a pointer to a non-pointer type is something which you should
never do. Instead, do something a bit more convoluted, but which is
guaranteed to give us the right pointer, as well as making sure that the
memory we exchange between python/cffi and the C extension is of the
right pointer size.

While we're touching this code, fix which object we pass to the Remote
constructor to keep alive. We need to pass the Repository object to
stop it from becoming unreferenced (and thus freeing memory the remote
needs) instead of the git_repository pointer.
This commit is contained in:
Carlos Martín Nieto
2014-04-14 02:13:36 +02:00
parent f3bb8a4556
commit 5a20510f8a
2 changed files with 16 additions and 18 deletions

View File

@@ -41,6 +41,15 @@ from .remote import Remote
class Repository(_Repository):
def __init__(self, *args, **kwargs):
super(Repository, self).__init__(*args, **kwargs)
# Get the pointer as the contents of a buffer and store it for
# later access
repo_cptr = ffi.new('git_repository **')
ffi.buffer(repo_cptr)[:] = self._pointer[:]
self._repo = repo_cptr[0]
#
# Mapping interface
#
@@ -72,36 +81,30 @@ class Repository(_Repository):
Creates a new remote.
"""
repo_cptr = ffi.new('git_repository **')
repo_cptr[0] = ffi.cast('git_repository *', self._pointer)
cremote = ffi.new('git_remote **')
repo = repo_cptr[0]
err = C.git_remote_create(cremote, repo, to_str(name), to_str(url))
err = C.git_remote_create(cremote, self._repo, to_str(name), to_str(url))
check_error(err)
return Remote(repo, cremote[0])
return Remote(self, cremote[0])
@property
def remotes(self):
"""Returns all configured remotes"""
repo_cptr = ffi.new('git_repository **')
repo_cptr[0] = ffi.cast('git_repository *', self._pointer)
names = ffi.new('git_strarray *')
repo = repo_cptr[0]
try:
err = C.git_remote_list(names, repo)
err = C.git_remote_list(names, self._repo)
check_error(err)
l = [None] * names.count
cremote = ffi.new('git_remote **')
for i in range(names.count):
err = C.git_remote_load(cremote, repo, names.strings[i])
err = C.git_remote_load(cremote, self._repo, names.strings[i])
check_error(err)
l[i] = Remote(repo, cremote[0])
l[i] = Remote(self, cremote[0])
return l
finally:
C.git_strarray_free(names)

View File

@@ -1293,13 +1293,8 @@ PyDoc_STRVAR(Repository__pointer__doc__, "Get the repo's pointer. For internal u
PyObject *
Repository__pointer__get__(Repository *self)
{
/*
* This is pretty bad. We shouldn't be casting a pointer into an
* integer, but we can't access the contents of a PyCapsule from
* python code, which we need to do in order to get a type that
* cffi likes.
*/
return PyLong_FromLongLong((long long) self->repo);
/* Bytes means a raw buffer */
return PyBytes_FromStringAndSize((char *) &self->repo, sizeof(git_repository *));
}
PyDoc_STRVAR(Repository_checkout_head__doc__,