173 lines
6.1 KiB
Diff
173 lines
6.1 KiB
Diff
From 6b935d6b53fdfcf6d83a0016487da807ed3a9139 Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= <cmn@dwim.me>
|
|
Date: Fri, 6 Feb 2015 03:41:37 +0100
|
|
Subject: [PATCH] Safer handling of string arrays
|
|
|
|
We need to keep hold of the strings which we create. We must also hold
|
|
on to the array of strings which we assing to our git_strarray.
|
|
|
|
We were not doing the latter, which meant that our strings may have been
|
|
freed too early, leaving us with with memory access errors (though often
|
|
not leading to a crash due to the custom allocator in python).
|
|
|
|
As we need to keep hold of two/three pieces of information, this looks
|
|
like a good place to introduce a context manager. This allows us to
|
|
keep these pointers alive without burdening the call sites with a return
|
|
of multiple objects they have no use for.
|
|
---
|
|
pygit2/index.py | 8 ++++----
|
|
pygit2/remote.py | 20 ++++++++++----------
|
|
pygit2/utils.py | 46 +++++++++++++++++++++++-----------------------
|
|
3 files changed, 37 insertions(+), 37 deletions(-)
|
|
|
|
diff --git a/pygit2/index.py b/pygit2/index.py
|
|
index 61b7c97..47d4cd6 100644
|
|
--- a/pygit2/index.py
|
|
+++ b/pygit2/index.py
|
|
@@ -32,7 +32,7 @@ from __future__ import absolute_import, unicode_literals
|
|
from _pygit2 import Oid, Tree, Diff
|
|
from .errors import check_error
|
|
from .ffi import ffi, C
|
|
-from .utils import is_string, strings_to_strarray, to_bytes, to_str
|
|
+from .utils import is_string, to_bytes, to_str, StrArray
|
|
|
|
|
|
class Index(object):
|
|
@@ -175,9 +175,9 @@ class Index(object):
|
|
If pathspecs are specified, only files matching those pathspecs will
|
|
be added.
|
|
"""
|
|
- arr, refs = strings_to_strarray(pathspecs)
|
|
- err = C.git_index_add_all(self._index, arr, 0, ffi.NULL, ffi.NULL)
|
|
- check_error(err, True)
|
|
+ with StrArray(pathspecs) as arr:
|
|
+ err = C.git_index_add_all(self._index, arr, 0, ffi.NULL, ffi.NULL)
|
|
+ check_error(err, True)
|
|
|
|
def add(self, path_or_entry):
|
|
"""add([path|entry])
|
|
diff --git a/pygit2/remote.py b/pygit2/remote.py
|
|
index c4a195f..d2fbdcf 100644
|
|
--- a/pygit2/remote.py
|
|
+++ b/pygit2/remote.py
|
|
@@ -34,7 +34,7 @@ from .errors import check_error, GitError
|
|
from .ffi import ffi, C
|
|
from .credentials import KeypairFromAgent
|
|
from .refspec import Refspec
|
|
-from .utils import to_bytes, strarray_to_strings, strings_to_strarray
|
|
+from .utils import to_bytes, strarray_to_strings, StrArray
|
|
|
|
|
|
def maybe_string(ptr):
|
|
@@ -253,9 +253,9 @@ class Remote(object):
|
|
|
|
@fetch_refspecs.setter
|
|
def fetch_refspecs(self, l):
|
|
- arr, refs = strings_to_strarray(l)
|
|
- err = C.git_remote_set_fetch_refspecs(self._remote, arr)
|
|
- check_error(err)
|
|
+ with StrArray(l) as arr:
|
|
+ err = C.git_remote_set_fetch_refspecs(self._remote, arr)
|
|
+ check_error(err)
|
|
|
|
@property
|
|
def push_refspecs(self):
|
|
@@ -269,9 +269,9 @@ class Remote(object):
|
|
|
|
@push_refspecs.setter
|
|
def push_refspecs(self, l):
|
|
- arr, refs = strings_to_strarray(l)
|
|
- err = C.git_remote_set_push_refspecs(self._remote, arr)
|
|
- check_error(err)
|
|
+ with StrArray(l) as arr:
|
|
+ err = C.git_remote_set_push_refspecs(self._remote, arr)
|
|
+ check_error(err)
|
|
|
|
def add_fetch(self, spec):
|
|
"""add_fetch(refspec)
|
|
@@ -305,7 +305,6 @@ class Remote(object):
|
|
err = C.git_remote_init_callbacks(defaultcallbacks, 1)
|
|
check_error(err)
|
|
|
|
- refspecs, refspecs_refs = strings_to_strarray(specs)
|
|
if signature:
|
|
sig_cptr = ffi.new('git_signature **')
|
|
ffi.buffer(sig_cptr)[:] = signature._pointer[:]
|
|
@@ -333,8 +332,9 @@ class Remote(object):
|
|
raise
|
|
|
|
try:
|
|
- err = C.git_remote_push(self._remote, refspecs, ffi.NULL, sig_ptr, to_bytes(message))
|
|
- check_error(err)
|
|
+ with StrArray(specs) as refspecs:
|
|
+ err = C.git_remote_push(self._remote, refspecs, ffi.NULL, sig_ptr, to_bytes(message))
|
|
+ check_error(err)
|
|
finally:
|
|
self._self_handle = None
|
|
|
|
diff --git a/pygit2/utils.py b/pygit2/utils.py
|
|
index 3c5fc88..17582a4 100644
|
|
--- a/pygit2/utils.py
|
|
+++ b/pygit2/utils.py
|
|
@@ -50,34 +50,34 @@ def strarray_to_strings(arr):
|
|
return l
|
|
|
|
|
|
-def strings_to_strarray(l):
|
|
- """Convert a list of strings to a git_strarray
|
|
+class StrArray(object):
|
|
+ """A git_strarray wrapper
|
|
|
|
- We return first the git_strarray* you can pass to libgit2 and a
|
|
- list of references to the memory, which we must keep around for as
|
|
- long as the git_strarray must live.
|
|
- """
|
|
+ Use this in order to get a git_strarray* to pass to libgit2 out of a
|
|
+ list of strings. This has a context manager, which you should use, e.g.
|
|
|
|
- if not isinstance(l, list):
|
|
- raise TypeError("Value must be a list")
|
|
+ with StrArray(list_of_strings) as arr:
|
|
+ C.git_function_that_takes_strarray(arr)
|
|
+ """
|
|
|
|
- arr = ffi.new('git_strarray *')
|
|
- strings = ffi.new('char *[]', len(l))
|
|
+ def __init__(self, l):
|
|
+ if not isinstance(l, list):
|
|
+ raise TypeError("Value must be a list")
|
|
|
|
- # We need refs in order to keep a reference to the value returned
|
|
- # by the ffi.new(). Otherwise, they will be freed and the memory
|
|
- # re-used, with less than great consequences.
|
|
- refs = [None] * len(l)
|
|
+ arr = ffi.new('git_strarray *')
|
|
+ strings = [None] * len(l)
|
|
+ for i in range(len(l)):
|
|
+ if not is_string(l[i]):
|
|
+ raise TypeError("Value must be a string")
|
|
|
|
- for i in range(len(l)):
|
|
- if not is_string(l[i]):
|
|
- raise TypeError("Value must be a string")
|
|
+ strings[i] = ffi.new('char []', to_bytes(l[i]))
|
|
|
|
- s = ffi.new('char []', to_bytes(l[i]))
|
|
- refs[i] = s
|
|
- strings[i] = s
|
|
+ self._arr = ffi.new('char *[]', strings)
|
|
+ self._strings = strings
|
|
+ self.array = ffi.new('git_strarray *', [self._arr, len(strings)])
|
|
|
|
- arr.strings = strings
|
|
- arr.count = len(l)
|
|
+ def __enter__(self):
|
|
+ return self.array
|
|
|
|
- return arr, refs
|
|
+ def __exit__(self, type, value, traceback):
|
|
+ pass
|
|
--
|
|
2.1.0
|
|
|