Re: [PATCH v2 3/8] python: Streamline initexception

[Nir Soffer]

On Mon, Jun 17, 2019 at 12:47 PM Pavel Bar  wrote:
> Just something to consider: *maybe* adding "finally" label at the end and
> freeing all the local variables using "Py_CLEAR" in the reverse order of
> their initialization and always returning "excp" (initialized to NULL at
> the beginning) could be more readable. Just a thought.

The point of this patch is to remove the cleanup. Why keep the local
variable when we can get
rid of them quickly?

On Sun, Jun 16, 2019 at 11:50 PM Nir Soffer  wrote:
> > When we create the sanlock expcetion we create a function, create a
>
> expcetion --> exception
> > method from the function, create a dict from the method, and finally
> > create an exception from the dict. This simple flow does not require
> > goto for cleanup.
> >
> > Creating a dict is simpler and less error prone using Py_BuildValue().
> >
> > When we finish with unused pointers, it is safer to set them to NULL
> > using Py_CLEAR(), ensuring that we don't access freed memory.
> > Signed-off-by: Nir Soffer
> >
> > python/sanlock.c | 31 ++++++++++++-------------------
> >  1 file changed, 12 insertions(+), 19 deletions(-)
> >
> > diff --git a/python/sanlock.c b/python/sanlock.c
> > index 21a7a76..5c2fd1a 100644
> > --- a/python/sanlock.c
> > +++ b/python/sanlock.c
> > @@ -1741,33 +1741,26 @@ sanlock_exception = {
> >  };
> >
> > static PyObject *
> >  initexception(void)
> >  {
> > int rv;
> > PyObject *dict, *func, *meth, *excp = NULL;
> > if ((dict = PyDict_New()) == NULL)
> >    goto exit_fail;
> > if ((func = PyCFunction_New(&sanlock_exception, NULL)) == NULL)
> >    goto exit_fail;
> > PyObject *func = PyCFunction_New(&sanlock_exception, NULL);
> > if (func == NULL)
> >    return NULL;
> >
> > meth = PyObject_CallFunction((PyObject *) &PyProperty_Type, "O",
> >
> > func);
> > Py_DECREF(func);
> > PyObject *meth = PyObject_CallFunction((PyObject *)
> >
> > &PyProperty_Type, "O", func);
> > Py_CLEAR(func);
> > if (meth == NULL)
> >    goto exit_fail;
> > rv = PyDict_SetItemString(dict, sanlock_exception.ml_name, meth);
> > Py_DECREF(meth);
> > if (rv < 0)
> >    goto exit_fail;
> >    return NULL;
> >
> > excp = PyErr_NewException("sanlock.SanlockException", NULL, dict);
> > PyObject *dict = Py_BuildValue("{s:O}", sanlock_exception.ml_name,
> >
> > meth);
> > Py_CLEAR(meth);
> > if (dict == NULL)
> >    return NULL;
> >
> > -exit_fail:
> > Py_XDECREF(dict);
> > PyObject *excp = PyErr_NewException("sanlock.SanlockException",
> >
> > NULL, dict);
> >
> > Py_CLEAR(dict);
> >
> > return excp;
> >
> > }
> > static int
> >
> > 2.17.2