Safer handling of string arrays

pygit2/index.py

@@ -32,7 +32,7 @@
 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 @@ def add_all(self, pathspecs=[]):
         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])

pygit2/remote.py

@@ -34,7 +34,7 @@
 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 @@ def fetch_refspecs(self):
 
     @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 @@ def push_refspecs(self):
 
     @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 @@ def push(self, specs, signature=None, message=None):
         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 @@ def push(self, specs, signature=None, message=None):
             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
 

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