Hi,
òÏÍÁÎ äÏÎÞÅÎËÏ wrote:
> static svn_error_t *callback_exception_error(void)
> {
> PyObject *svn_module = NULL, *svn_exc = NULL;
> PyObject *message = NULL, *apr_err = NULL;
> PyObject *exc, *exc_type, *exc_traceback;
> svn_error_t *rv = NULL;
>
> PyErr_Fetch(&exc_type, &exc, &exc_traceback);
>
> if ((svn_module = PyImport_ImportModule("svn.core")) == NULL)
> goto finished;
If we take this goto, does this code leak references to EXC_TYPE,
EXC, and EXC_TRACEBACK?
> if ((svn_exc = PyObject_GetAttrString(svn_module, "SubversionException"))
> == NULL)
> goto finished;
Ditto.
>
> if (PyErr_GivenExceptionMatches(exc_type, svn_exc))
> {
> Py_DECREF(exc_type);
> Py_DECREF(exc_traceback);
Why not move these two lines after the if() statement?
That way, you can negate the sense of the if() statement
and avoid having an ELSE block. I.e.:
if (!PyErr_GivenExceptionMatches(exc_type, svn_exc))
{
PyErr_Restore(exc_type, exc, exc_traceback);
goto finished;
}
Py_DECREF(exc_type);
Py_DECREF(exc_traceback);
> }
> else
> {
> PyErr_Restore(exc_type, exc, exc_traceback);
> goto finished;
> }
>
> if ((apr_err = PyObject_GetAttrString(exc, "apr_err")) == NULL)
> goto finished;
Here, if we take the goto, is EXC leaked?
> if ((message = PyObject_GetAttrString(exc, "message")) == NULL)
> goto finished;
Ditto.
>
> Py_DECREF(exc);
>
> /* A possible improvement here would be to convert the whole
> SubversionException chain. */
> if (PyString_Check(message) && PyInt_Check(apr_err))
> rv = svn_error_create(PyInt_AsLong(apr_err), NULL,
> PyString_AsString(message));
If this if() statement is false, what happens? Does this
raise the SVN error SVN_ERR_SWIG_PY_EXCEPTION_SET,
but without there being a Python exception set? (Looking in
the Python 2.6.2 source code, it looks like PyInt_Check() is a
macro that expands to some bitfield-checking, it doesn't set
a Python exception for you).
>
> finished:
> Py_XDECREF(svn_module);
> Py_XDECREF(svn_exc);
> Py_XDECREF(apr_err);
> Py_XDECREF(message);
> return rv ? rv : svn_error_create(SVN_ERR_SWIG_PY_EXCEPTION_SET, NULL,
> "Python callback raised an exception");
> }
Kind regards,
Jon
**********************************************************************
This email and its attachments may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Cabot Communications Ltd.
If you are not the intended recipient of this email and its attachments, you must take no action based upon them, nor must you copy or show them to anyone.
Cabot Communications Limited
Verona House, Filwood Road, Bristol BS16 3RY, UK
+44 (0) 1179584232
Co. Registered in England number 02817269
Please contact the sender if you believe you have received this email in error.
**********************************************************************
______________________________________________________________________
This email has been scanned by the MessageLabs Email Security System.
For more information please visit http://www.messagelabs.com/email
______________________________________________________________________
Received on 2010-03-24 13:42:23 CET