python: Rework callbacks.
authorJustus Winter <justus@g10code.com>
Mon, 13 Jun 2016 17:16:30 +0000 (19:16 +0200)
committerJustus Winter <justus@g10code.com>
Thu, 16 Jun 2016 12:19:17 +0000 (14:19 +0200)
Simplify how the lifetime of callback arguments is managed.

* lang/python/gpgme.i (gpgme_edit_cb_t): Check arguments.
(PyObject_p_p, void_p_p): Drop rather dangerous interface.
(pygpgme_unwrap_gpgme_ctx_t): New function.
* lang/python/helpers.c (pygpgme_clear_generic_cb): Drop dangerous
function.
(pyPassphraseCb): Assert contract.
(pygpgme_set_passphrase_cb): Use Python's calling convention so that
we can raise exceptions.  Hand in 'self', get the wrapped object, and
simply store the hook data as attribute of the wrapper object.
(pyProgressCb, pygpgme_set_progress_cb): Likewise.
(pygpgme_set_status_cb): Likewise.
(pygpgme_data_new_from_cbs): Likewise.
* lang/python/helpers.h (pygpgme_clear_generic_cb): Drop prototype.
(pygpgme_set_passphrase_cb): Update prototype.
(pygpgme_set_progress_cb): Likewise.
(pygpgme_set_status_cb): Likewise.
(pygpgme_data_new_from_cbs): Likewise.
(pygpgme_unwrap_gpgme_ctx_t): New prottotype.
* lang/python/pyme/core.py (Context, Data): Update callsites.

Signed-off-by: Justus Winter <justus@g10code.com>
lang/python/gpgme.i
lang/python/helpers.c
lang/python/helpers.h
lang/python/pyme/core.py

index 1e4c9ff..65cd235 100644 (file)
 
 // Include mapper for edit callbacks
 %typemap(in) (gpgme_edit_cb_t fnc, void *fnc_value) {
 
 // Include mapper for edit callbacks
 %typemap(in) (gpgme_edit_cb_t fnc, void *fnc_value) {
+  if (! PyTuple_Check($input))
+    return PyErr_Format(PyExc_TypeError, "edit callback must be a tuple");
+  if (PyTuple_Size($input) != 2 && PyTuple_Size($input) != 3)
+    return PyErr_Format(PyExc_TypeError,
+                        "edit callback must be a tuple of size 2 or 3");
+
   $1 = (gpgme_edit_cb_t) pyEditCb;
   $1 = (gpgme_edit_cb_t) pyEditCb;
-  if ($input == Py_None)
-    $2 = NULL;
-  else
-    $2 = $input;
+  $2 = $input;
 }
 
 /* Include the unmodified <gpgme.h> for cc, and the cleaned-up local
 }
 
 /* Include the unmodified <gpgme.h> for cc, and the cleaned-up local
@@ -355,8 +358,6 @@ struct _gpgme_sig_notation
 %pointer_functions(gpgme_error_t, gpgme_error_t_p);
 %pointer_functions(gpgme_trust_item_t, gpgme_trust_item_t_p);
 %pointer_functions(gpgme_engine_info_t, gpgme_engine_info_t_p);
 %pointer_functions(gpgme_error_t, gpgme_error_t_p);
 %pointer_functions(gpgme_trust_item_t, gpgme_trust_item_t_p);
 %pointer_functions(gpgme_engine_info_t, gpgme_engine_info_t_p);
-%pointer_functions(PyObject *, PyObject_p_p);
-%pointer_functions(void *, void_p_p);
 
 // Helper functions.
 
 
 // Helper functions.
 
@@ -374,6 +375,18 @@ pygpgme_wrap_gpgme_data_t(gpgme_data_t data)
 {
   return SWIG_NewPointerObj(data, SWIGTYPE_p_gpgme_data, 0);
 }
 {
   return SWIG_NewPointerObj(data, SWIGTYPE_p_gpgme_data, 0);
 }
+
+gpgme_ctx_t
+pygpgme_unwrap_gpgme_ctx_t(PyObject *wrapped)
+{
+  gpgme_ctx_t result;
+  if (SWIG_ConvertPtr(wrapped,
+                      (void **) &result,
+                      SWIGTYPE_p_gpgme_context,
+                      SWIG_POINTER_EXCEPTION) == -1)
+    return NULL;
+  return result;
+}
 %}
 
 %include "helpers.h"
 %}
 
 %include "helpers.h"
index ad33d07..5380ff2 100644 (file)
@@ -76,10 +76,6 @@ gpgme_error_t pygpgme_exception2code(void) {
   return err_status;
 }
 
   return err_status;
 }
 
-void pygpgme_clear_generic_cb(PyObject **cb) {
-  Py_DECREF(*cb);
-}
-
 /* Exception support for callbacks.  */
 #define EXCINFO        "_callback_excinfo"
 
 /* Exception support for callbacks.  */
 #define EXCINFO        "_callback_excinfo"
 
@@ -293,6 +289,7 @@ static gpgme_error_t pyPassphraseCb(void *hook,
   pygpgme_exception_init();
 
   assert (PyTuple_Check(pyhook));
   pygpgme_exception_init();
 
   assert (PyTuple_Check(pyhook));
+  assert (PyTuple_Size(pyhook) == 2 || PyTuple_Size(pyhook) == 3);
   self = PyTuple_GetItem(pyhook, 0);
   func = PyTuple_GetItem(pyhook, 1);
   if (PyTuple_Size(pyhook) == 3) {
   self = PyTuple_GetItem(pyhook, 0);
   func = PyTuple_GetItem(pyhook, 1);
   if (PyTuple_Size(pyhook) == 3) {
@@ -374,15 +371,47 @@ static gpgme_error_t pyPassphraseCb(void *hook,
   return err_status;
 }
 
   return err_status;
 }
 
-void pygpgme_set_passphrase_cb(gpgme_ctx_t ctx, PyObject *cb,
-                              PyObject **freelater) {
+PyObject *
+pygpgme_set_passphrase_cb(PyObject *self, PyObject *cb) {
+  PyObject *wrapped;
+  gpgme_ctx_t ctx;
+
+  wrapped = PyObject_GetAttrString(self, "wrapped");
+  if (wrapped == NULL)
+    {
+      assert (PyErr_Occurred ());
+      return NULL;
+    }
+
+  ctx = pygpgme_unwrap_gpgme_ctx_t(wrapped);
+  Py_DECREF(wrapped);
+  if (ctx == NULL)
+    {
+      if (cb == Py_None)
+        goto out;
+      else
+        return PyErr_Format(PyExc_RuntimeError, "wrapped is NULL");
+    }
+
   if (cb == Py_None) {
     gpgme_set_passphrase_cb(ctx, NULL, NULL);
   if (cb == Py_None) {
     gpgme_set_passphrase_cb(ctx, NULL, NULL);
-    return;
+    PyObject_SetAttrString(self, "_passphrase_cb", Py_None);
+    goto out;
   }
   }
-  Py_INCREF(cb);
-  *freelater = cb;
-  gpgme_set_passphrase_cb(ctx, (gpgme_passphrase_cb_t)pyPassphraseCb, (void *) cb);
+
+  if (! PyTuple_Check(cb))
+    return PyErr_Format(PyExc_TypeError, "cb must be a tuple");
+  if (PyTuple_Size(cb) != 2 && PyTuple_Size(cb) != 3)
+    return PyErr_Format(PyExc_TypeError,
+                        "cb must be a tuple of size 2 or 3");
+
+  gpgme_set_passphrase_cb(ctx, (gpgme_passphrase_cb_t) pyPassphraseCb,
+                          (void *) cb);
+  PyObject_SetAttrString(self, "_passphrase_cb", cb);
+
+ out:
+  Py_INCREF(Py_None);
+  return Py_None;
 }
 
 static void pyProgressCb(void *hook, const char *what, int type, int current,
 }
 
 static void pyProgressCb(void *hook, const char *what, int type, int current,
@@ -392,6 +421,7 @@ static void pyProgressCb(void *hook, const char *what, int type, int current,
   PyObject *self = NULL;
 
   assert (PyTuple_Check(pyhook));
   PyObject *self = NULL;
 
   assert (PyTuple_Check(pyhook));
+  assert (PyTuple_Size(pyhook) == 2 || PyTuple_Size(pyhook) == 3);
   self = PyTuple_GetItem(pyhook, 0);
   func = PyTuple_GetItem(pyhook, 1);
   if (PyTuple_Size(pyhook) == 3) {
   self = PyTuple_GetItem(pyhook, 0);
   func = PyTuple_GetItem(pyhook, 1);
   if (PyTuple_Size(pyhook) == 3) {
@@ -423,14 +453,46 @@ static void pyProgressCb(void *hook, const char *what, int type, int current,
   Py_XDECREF(retval);
 }
 
   Py_XDECREF(retval);
 }
 
-void pygpgme_set_progress_cb(gpgme_ctx_t ctx, PyObject *cb, PyObject **freelater){
+PyObject *
+pygpgme_set_progress_cb(PyObject *self, PyObject *cb) {
+  PyObject *wrapped;
+  gpgme_ctx_t ctx;
+
+  wrapped = PyObject_GetAttrString(self, "wrapped");
+  if (wrapped == NULL)
+    {
+      assert (PyErr_Occurred ());
+      return NULL;
+    }
+
+  ctx = pygpgme_unwrap_gpgme_ctx_t(wrapped);
+  Py_DECREF(wrapped);
+  if (ctx == NULL)
+    {
+      if (cb == Py_None)
+        goto out;
+      else
+        return PyErr_Format(PyExc_RuntimeError, "wrapped is NULL");
+    }
+
   if (cb == Py_None) {
     gpgme_set_progress_cb(ctx, NULL, NULL);
   if (cb == Py_None) {
     gpgme_set_progress_cb(ctx, NULL, NULL);
-    return;
+    PyObject_SetAttrString(self, "_progress_cb", Py_None);
+    goto out;
   }
   }
-  Py_INCREF(cb);
-  *freelater = cb;
+
+  if (! PyTuple_Check(cb))
+    return PyErr_Format(PyExc_TypeError, "cb must be a tuple");
+  if (PyTuple_Size(cb) != 2 && PyTuple_Size(cb) != 3)
+    return PyErr_Format(PyExc_TypeError,
+                        "cb must be a tuple of size 2 or 3");
+
   gpgme_set_progress_cb(ctx, (gpgme_progress_cb_t) pyProgressCb, (void *) cb);
   gpgme_set_progress_cb(ctx, (gpgme_progress_cb_t) pyProgressCb, (void *) cb);
+  PyObject_SetAttrString(self, "_progress_cb", cb);
+
+ out:
+  Py_INCREF(Py_None);
+  return Py_None;
 }
 \f
 /* Status callbacks.  */
 }
 \f
 /* Status callbacks.  */
@@ -488,15 +550,46 @@ static gpgme_error_t pyStatusCb(void *hook, const char *keyword,
   return err;
 }
 
   return err;
 }
 
-void pygpgme_set_status_cb(gpgme_ctx_t ctx, PyObject *cb,
-                           PyObject **freelater) {
+PyObject *
+pygpgme_set_status_cb(PyObject *self, PyObject *cb) {
+  PyObject *wrapped;
+  gpgme_ctx_t ctx;
+
+  wrapped = PyObject_GetAttrString(self, "wrapped");
+  if (wrapped == NULL)
+    {
+      assert (PyErr_Occurred ());
+      return NULL;
+    }
+
+  ctx = pygpgme_unwrap_gpgme_ctx_t(wrapped);
+  Py_DECREF(wrapped);
+  if (ctx == NULL)
+    {
+      if (cb == Py_None)
+        goto out;
+      else
+        return PyErr_Format(PyExc_RuntimeError, "wrapped is NULL");
+    }
+
   if (cb == Py_None) {
     gpgme_set_status_cb(ctx, NULL, NULL);
   if (cb == Py_None) {
     gpgme_set_status_cb(ctx, NULL, NULL);
-    return;
+    PyObject_SetAttrString(self, "_status_cb", Py_None);
+    goto out;
   }
   }
-  Py_INCREF(cb);
-  *freelater = cb;
+
+  if (! PyTuple_Check(cb))
+    return PyErr_Format(PyExc_TypeError, "cb must be a tuple");
+  if (PyTuple_Size(cb) != 2 && PyTuple_Size(cb) != 3)
+    return PyErr_Format(PyExc_TypeError,
+                        "cb must be a tuple of size 2 or 3");
+
   gpgme_set_status_cb(ctx, (gpgme_status_cb_t) pyStatusCb, (void *) cb);
   gpgme_set_status_cb(ctx, (gpgme_status_cb_t) pyStatusCb, (void *) cb);
+  PyObject_SetAttrString(self, "_status_cb", cb);
+
+ out:
+  Py_INCREF(Py_None);
+  return Py_None;
 }
 \f
 /* Edit callbacks.  */
 }
 \f
 /* Edit callbacks.  */
@@ -775,9 +868,10 @@ static void pyDataReleaseCb(void *hook)
     pygpgme_stash_callback_exception(self);
 }
 
     pygpgme_stash_callback_exception(self);
 }
 
-gpgme_error_t pygpgme_data_new_from_cbs(gpgme_data_t *r_data,
-                                        PyObject *pycbs,
-                                        PyObject **freelater)
+PyObject *
+pygpgme_data_new_from_cbs(PyObject *self,
+                          PyObject *pycbs,
+                          gpgme_data_t *r_data)
 {
   static struct gpgme_data_cbs cbs = {
     pyDataReadCb,
 {
   static struct gpgme_data_cbs cbs = {
     pyDataReadCb,
@@ -785,12 +879,20 @@ gpgme_error_t pygpgme_data_new_from_cbs(gpgme_data_t *r_data,
     pyDataSeekCb,
     pyDataReleaseCb,
   };
     pyDataSeekCb,
     pyDataReleaseCb,
   };
+  gpgme_error_t err;
+
+  if (! PyTuple_Check(pycbs))
+    return PyErr_Format(PyExc_TypeError, "pycbs must be a tuple");
+  if (PyTuple_Size(pycbs) != 5 && PyTuple_Size(pycbs) != 6)
+    return PyErr_Format(PyExc_TypeError,
+                        "pycbs must be a tuple of size 5 or 6");
 
 
-  assert (PyTuple_Check(pycbs));
-  assert (PyTuple_Size(pycbs) == 5 || PyTuple_Size(pycbs) == 6);
+  err = gpgme_data_new_from_cbs(r_data, &cbs, (void *) pycbs);
+  if (err)
+    return pygpgme_raise_exception(err);
 
 
-  Py_INCREF(pycbs);
-  *freelater = pycbs;
+  PyObject_SetAttrString(self, "_data_cbs", pycbs);
 
 
-  return gpgme_data_new_from_cbs(r_data, &cbs, (void *) pycbs);
+  Py_INCREF(Py_None);
+  return Py_None;
 }
 }
index 37362ae..1564290 100644 (file)
@@ -34,21 +34,18 @@ PyObject *object_to_gpgme_data_t(PyObject *input, int argnum,
                                 gpgme_data_t *wrapper,
                                 PyObject **bytesio, Py_buffer *view);
 
                                 gpgme_data_t *wrapper,
                                 PyObject **bytesio, Py_buffer *view);
 
-void pygpgme_clear_generic_cb(PyObject **cb);
 PyObject *pygpgme_raise_callback_exception(PyObject *self);
 
 PyObject *pygpgme_raise_callback_exception(PyObject *self);
 
-void pygpgme_set_passphrase_cb(gpgme_ctx_t ctx, PyObject *cb,
-                              PyObject **freelater);
-void pygpgme_set_progress_cb(gpgme_ctx_t ctx, PyObject *cb, PyObject **freelater);
-void pygpgme_set_status_cb(gpgme_ctx_t ctx, PyObject *cb,
-                           PyObject **freelater);
+PyObject *pygpgme_set_passphrase_cb(PyObject *self, PyObject *cb);
+PyObject *pygpgme_set_progress_cb(PyObject *self, PyObject *cb);
+PyObject *pygpgme_set_status_cb(PyObject *self, PyObject *cb);
 
 gpgme_error_t pyEditCb(void *opaque, gpgme_status_code_t status,
                       const char *args, int fd);
 
 
 gpgme_error_t pyEditCb(void *opaque, gpgme_status_code_t status,
                       const char *args, int fd);
 
-gpgme_error_t pygpgme_data_new_from_cbs(gpgme_data_t *r_data,
-                                        PyObject *pycbs,
-                                        PyObject **freelater);
+PyObject *pygpgme_data_new_from_cbs(PyObject *self, PyObject *pycbs,
+                                   gpgme_data_t *r_data);
 
 /* SWIG support for helpers.c  */
 PyObject *pygpgme_wrap_gpgme_data_t(gpgme_data_t data);
 
 /* SWIG support for helpers.c  */
 PyObject *pygpgme_wrap_gpgme_data_t(gpgme_data_t data);
+gpgme_ctx_t pygpgme_unwrap_gpgme_ctx_t(PyObject *wrapped);
index 64dc787..e5a5061 100644 (file)
@@ -220,9 +220,6 @@ class Context(GpgmeWrapper):
             pygpgme.delete_gpgme_ctx_t_p(tmp)
             self.own = True
         super().__init__(wrapped)
             pygpgme.delete_gpgme_ctx_t_p(tmp)
             self.own = True
         super().__init__(wrapped)
-        self.last_passcb = None
-        self.last_progresscb = None
-        self.last_statuscb = None
         self.armor = armor
         self.textmode = textmode
         self.offline = offline
         self.armor = armor
         self.textmode = textmode
         self.offline = offline
@@ -247,30 +244,6 @@ class Context(GpgmeWrapper):
     def __exit__(self, type, value, tb):
         self.__del__()
 
     def __exit__(self, type, value, tb):
         self.__del__()
 
-    def _free_passcb(self):
-        if self.last_passcb != None:
-            if pygpgme.pygpgme_clear_generic_cb:
-                pygpgme.pygpgme_clear_generic_cb(self.last_passcb)
-            if pygpgme.delete_PyObject_p_p:
-                pygpgme.delete_PyObject_p_p(self.last_passcb)
-            self.last_passcb = None
-
-    def _free_progresscb(self):
-        if self.last_progresscb != None:
-            if pygpgme.pygpgme_clear_generic_cb:
-                pygpgme.pygpgme_clear_generic_cb(self.last_progresscb)
-            if pygpgme.delete_PyObject_p_p:
-                pygpgme.delete_PyObject_p_p(self.last_progresscb)
-            self.last_progresscb = None
-
-    def _free_statuscb(self):
-        if self.last_statuscb != None:
-            if pygpgme.pygpgme_clear_generic_cb:
-                pygpgme.pygpgme_clear_generic_cb(self.last_statuscb)
-            if pygpgme.delete_PyObject_p_p:
-                pygpgme.delete_PyObject_p_p(self.last_statuscb)
-            self.last_statuscb = None
-
     def op_keylist_all(self, *args, **kwargs):
         self.op_keylist_start(*args, **kwargs)
         key = self.op_keylist_next()
     def op_keylist_all(self, *args, **kwargs):
         self.op_keylist_start(*args, **kwargs)
         key = self.op_keylist_next()
@@ -341,16 +314,18 @@ class Context(GpgmeWrapper):
 
         Please see the GPGME manual for more information.
         """
 
         Please see the GPGME manual for more information.
         """
-        self._free_passcb()
         if func == None:
             hookdata = None
         else:
         if func == None:
             hookdata = None
         else:
-            self.last_passcb = pygpgme.new_PyObject_p_p()
             if hook == None:
                 hookdata = (weakref.ref(self), func)
             else:
                 hookdata = (weakref.ref(self), func, hook)
             if hook == None:
                 hookdata = (weakref.ref(self), func)
             else:
                 hookdata = (weakref.ref(self), func, hook)
-        pygpgme.pygpgme_set_passphrase_cb(self.wrapped, hookdata, self.last_passcb)
+        pygpgme.pygpgme_set_passphrase_cb(self, hookdata)
+
+    def _free_passcb(self):
+        if pygpgme.pygpgme_set_passphrase_cb:
+            self.set_passphrase_cb(None)
 
     def set_progress_cb(self, func, hook=None):
         """Sets the progress meter callback to the function specified by FUNC.
 
     def set_progress_cb(self, func, hook=None):
         """Sets the progress meter callback to the function specified by FUNC.
@@ -364,16 +339,18 @@ class Context(GpgmeWrapper):
         Please see the GPGME manual for more information.
 
         """
         Please see the GPGME manual for more information.
 
         """
-        self._free_progresscb()
         if func == None:
             hookdata = None
         else:
         if func == None:
             hookdata = None
         else:
-            self.last_progresscb = pygpgme.new_PyObject_p_p()
             if hook == None:
                 hookdata = (weakref.ref(self), func)
             else:
                 hookdata = (weakref.ref(self), func, hook)
             if hook == None:
                 hookdata = (weakref.ref(self), func)
             else:
                 hookdata = (weakref.ref(self), func, hook)
-        pygpgme.pygpgme_set_progress_cb(self.wrapped, hookdata, self.last_progresscb)
+        pygpgme.pygpgme_set_progress_cb(self, hookdata)
+
+    def _free_progresscb(self):
+        if pygpgme.pygpgme_set_progress_cb:
+            self.set_progress_cb(None)
 
     def set_status_cb(self, func, hook=None):
         """Sets the status callback to the function specified by FUNC.  If
 
     def set_status_cb(self, func, hook=None):
         """Sets the status callback to the function specified by FUNC.  If
@@ -386,17 +363,18 @@ class Context(GpgmeWrapper):
         Please see the GPGME manual for more information.
 
         """
         Please see the GPGME manual for more information.
 
         """
-        self._free_statuscb()
         if func == None:
             hookdata = None
         else:
         if func == None:
             hookdata = None
         else:
-            self.last_statuscb = pygpgme.new_PyObject_p_p()
             if hook == None:
                 hookdata = (weakref.ref(self), func)
             else:
                 hookdata = (weakref.ref(self), func, hook)
             if hook == None:
                 hookdata = (weakref.ref(self), func)
             else:
                 hookdata = (weakref.ref(self), func, hook)
-        pygpgme.pygpgme_set_status_cb(self.wrapped, hookdata,
-                                      self.last_statuscb)
+        pygpgme.pygpgme_set_status_cb(self, hookdata)
+
+    def _free_statuscb(self):
+        if pygpgme.pygpgme_set_status_cb:
+            self.set_status_cb(None)
 
     def get_engine_info(self):
         """Returns this context specific engine info"""
 
     def get_engine_info(self):
         """Returns this context specific engine info"""
@@ -547,12 +525,7 @@ class Data(GpgmeWrapper):
         self.__del__()
 
     def _free_datacbs(self):
         self.__del__()
 
     def _free_datacbs(self):
-        if self.data_cbs != None:
-            if pygpgme.pygpgme_clear_generic_cb:
-                pygpgme.pygpgme_clear_generic_cb(self.data_cbs)
-            if pygpgme.delete_PyObject_p_p:
-                pygpgme.delete_PyObject_p_p(self.data_cbs)
-            self.data_cbs = None
+        self._data_cbs = None
 
     def new(self):
         tmp = pygpgme.new_gpgme_data_t_p()
 
     def new(self):
         tmp = pygpgme.new_gpgme_data_t_p()
@@ -579,8 +552,6 @@ class Data(GpgmeWrapper):
         pygpgme.delete_gpgme_data_t_p(tmp)
 
     def new_from_cbs(self, read_cb, write_cb, seek_cb, release_cb, hook=None):
         pygpgme.delete_gpgme_data_t_p(tmp)
 
     def new_from_cbs(self, read_cb, write_cb, seek_cb, release_cb, hook=None):
-        assert self.data_cbs == None
-        self.data_cbs = pygpgme.new_PyObject_p_p()
         tmp = pygpgme.new_gpgme_data_t_p()
         if hook != None:
             hookdata = (weakref.ref(self),
         tmp = pygpgme.new_gpgme_data_t_p()
         if hook != None:
             hookdata = (weakref.ref(self),
@@ -588,8 +559,7 @@ class Data(GpgmeWrapper):
         else:
             hookdata = (weakref.ref(self),
                         read_cb, write_cb, seek_cb, release_cb)
         else:
             hookdata = (weakref.ref(self),
                         read_cb, write_cb, seek_cb, release_cb)
-        errorcheck(
-            pygpgme.pygpgme_data_new_from_cbs(tmp, hookdata, self.data_cbs))
+        pygpgme.pygpgme_data_new_from_cbs(self, hookdata, tmp)
         self.wrapped = pygpgme.gpgme_data_t_p_value(tmp)
         pygpgme.delete_gpgme_data_t_p(tmp)
 
         self.wrapped = pygpgme.gpgme_data_t_p_value(tmp)
         pygpgme.delete_gpgme_data_t_p(tmp)