[SATYR PATCH v2] do not double-free list elements in python bindings

Jakub Filak jfilak at redhat.com
Mon Jul 15 11:06:05 UTC 2013


Pushed!

Please, follow the developers guidelines [1] next time:

- file a ticket (bugzilla, github issue)
- provide a valuable description of the problem because other team
members may not understand the root of cause of the problem and it can
help them when they hit a similar problem
- sign the patch


http://abrt.etherpad.corp.redhat.com/develguide


On Mon, 2013-07-15 at 12:45 +0200, Michal Toman wrote:
> ---
>  python/py_base_stacktrace.c   | 17 ----------
>  python/py_base_stacktrace.h   |  1 -
>  python/py_base_thread.c       | 18 ----------
>  python/py_base_thread.h       |  1 -
>  python/py_gdb_stacktrace.c    | 76 +++++++------------------------------------
>  python/py_gdb_thread.c        |  3 +-
>  python/py_java_stacktrace.c   |  3 +-
>  python/py_java_thread.c       |  3 +-
>  python/py_koops_stacktrace.c  | 10 +++---
>  python/py_python_stacktrace.c |  3 +-
>  10 files changed, 24 insertions(+), 111 deletions(-)
> 
> diff --git a/python/py_base_stacktrace.c b/python/py_base_stacktrace.c
> index 5739e3c..72d96f0 100644
> --- a/python/py_base_stacktrace.c
> +++ b/python/py_base_stacktrace.c
> @@ -205,23 +205,6 @@ threads_prepare_linked_list(struct sr_py_multi_stacktrace *stacktrace)
>      return 0;
>  }
>  
> -int
> -threads_free_python_list(struct sr_py_multi_stacktrace *stacktrace)
> -{
> -    int i;
> -    PyObject *item;
> -    for (i = 0; i < PyList_Size(stacktrace->threads); ++i)
> -    {
> -        item = PyList_GetItem(stacktrace->threads, i);
> -        if (!item)
> -            return -1;
> -        Py_DECREF(item);
> -    }
> -    Py_DECREF(stacktrace->threads);
> -
> -    return 0;
> -}
> -
>  PyObject *
>  threads_to_python_list(struct sr_stacktrace *stacktrace, PyTypeObject *thread_type, PyTypeObject *frame_type)
>  {
> diff --git a/python/py_base_stacktrace.h b/python/py_base_stacktrace.h
> index 6b12fa4..41eee62 100644
> --- a/python/py_base_stacktrace.h
> +++ b/python/py_base_stacktrace.h
> @@ -49,7 +49,6 @@ struct sr_py_multi_stacktrace
>  
>  /* helpers */
>  int threads_prepare_linked_list(struct sr_py_multi_stacktrace *stacktrace);
> -int threads_free_python_list(struct sr_py_multi_stacktrace *stacktrace);
>  PyObject *threads_to_python_list(struct sr_stacktrace *stacktrace,
>                                   PyTypeObject *thread_type, PyTypeObject *frame_type);
>  
> diff --git a/python/py_base_thread.c b/python/py_base_thread.c
> index 5bbbade..ff2eab7 100644
> --- a/python/py_base_thread.c
> +++ b/python/py_base_thread.c
> @@ -152,24 +152,6 @@ frames_prepare_linked_list(struct sr_py_base_thread *thread)
>      return 0;
>  }
>  
> -int
> -frames_free_python_list(struct sr_py_base_thread *thread)
> -{
> -    int i;
> -    PyObject *item;
> -
> -    for (i = 0; i < PyList_Size(thread->frames); ++i)
> -    {
> -        item = PyList_GetItem(thread->frames, i);
> -        if (!item)
> -            return -1;
> -        Py_DECREF(item);
> -    }
> -    Py_DECREF(thread->frames);
> -
> -    return 0;
> -}
> -
>  PyObject *
>  frames_to_python_list(struct sr_thread *thread, PyTypeObject *frame_type)
>  {
> diff --git a/python/py_base_thread.h b/python/py_base_thread.h
> index e9f1af7..aa3f1b7 100644
> --- a/python/py_base_thread.h
> +++ b/python/py_base_thread.h
> @@ -44,7 +44,6 @@ struct sr_py_base_thread
>  
>  /* helpers */
>  int frames_prepare_linked_list(struct sr_py_base_thread *thread);
> -int frames_free_python_list(struct sr_py_base_thread *thread);
>  PyObject *frames_to_python_list(struct sr_thread *thread, PyTypeObject *frame_type);
>  
>  int sr_py_base_thread_cmp(struct sr_py_base_thread *self, struct sr_py_base_thread *other);
> diff --git a/python/py_gdb_stacktrace.c b/python/py_gdb_stacktrace.c
> index bde65d2..4b47b94 100644
> --- a/python/py_gdb_stacktrace.c
> +++ b/python/py_gdb_stacktrace.c
> @@ -194,41 +194,6 @@ gdb_prepare_linked_lists(struct sr_py_gdb_stacktrace *stacktrace)
>      return 0;
>  }
>  
> -int
> -stacktrace_free_thread_python_list(struct sr_py_gdb_stacktrace *stacktrace)
> -{
> -    int i;
> -    PyObject *item;
> -    for (i = 0; i < PyList_Size(stacktrace->threads); ++i)
> -    {
> -        item = PyList_GetItem(stacktrace->threads, i);
> -        if (!item)
> -            return -1;
> -        Py_DECREF(item);
> -    }
> -    Py_DECREF(stacktrace->threads);
> -
> -    return 0;
> -}
> -
> -int
> -stacktrace_free_sharedlib_python_list(struct sr_py_gdb_stacktrace *stacktrace)
> -{
> -    int i;
> -    PyObject *item;
> -
> -    for (i = 0; i < PyList_Size(stacktrace->libs); ++i)
> -    {
> -        item = PyList_GetItem(stacktrace->libs, i);
> -        if (!item)
> -            return -1;
> -        Py_DECREF(item);
> -    }
> -    Py_DECREF(stacktrace->libs);
> -
> -    return 0;
> -}
> -
>  PyObject *
>  thread_linked_list_to_python_list(struct sr_gdb_stacktrace *stacktrace)
>  {
> @@ -294,17 +259,10 @@ stacktrace_rebuild_thread_python_list(struct sr_py_gdb_stacktrace *stacktrace)
>      struct sr_gdb_thread *newlinkedlist = sr_gdb_thread_dup(stacktrace->stacktrace->threads, true);
>      if (!newlinkedlist)
>          return -1;
> -    if (stacktrace_free_thread_python_list(stacktrace) < 0)
> -    {
> -        struct sr_gdb_thread *next;
> -        while (newlinkedlist)
> -        {
> -            next = newlinkedlist->next;
> -            sr_gdb_thread_free(newlinkedlist);
> -            newlinkedlist = next;
> -        }
> -        return -1;
> -    }
> +
> +    /* the list will decref all of its elements */
> +    Py_DECREF(stacktrace->threads);
> +
>      stacktrace->stacktrace->threads = newlinkedlist;
>      stacktrace->threads = threads_to_python_list((struct sr_stacktrace *)stacktrace->stacktrace,
>                                                   &sr_py_gdb_thread_type, &sr_py_gdb_frame_type);
> @@ -317,17 +275,9 @@ stacktrace_rebuild_sharedlib_python_list(struct sr_py_gdb_stacktrace *stacktrace
>      struct sr_gdb_sharedlib *newlinkedlist = sr_gdb_sharedlib_dup(stacktrace->stacktrace->libs, true);
>      if (!newlinkedlist)
>          return -1;
> -    if (stacktrace_free_sharedlib_python_list(stacktrace) < 0)
> -    {
> -        struct sr_gdb_sharedlib *next;
> -        while (newlinkedlist)
> -        {
> -            next = newlinkedlist->next;
> -            sr_gdb_sharedlib_free(newlinkedlist);
> -            newlinkedlist = next;
> -        }
> -        return -1;
> -    }
> +
> +    /* the list will decref all of its elements */
> +    Py_DECREF(stacktrace->libs);
>      stacktrace->stacktrace->libs = newlinkedlist;
>      stacktrace->libs = sharedlib_linked_list_to_python_list(stacktrace->stacktrace);
>      return 0;
> @@ -387,8 +337,9 @@ void
>  sr_py_gdb_stacktrace_free(PyObject *object)
>  {
>      struct sr_py_gdb_stacktrace *this = (struct sr_py_gdb_stacktrace*)object;
> -    stacktrace_free_thread_python_list(this);
> -    stacktrace_free_sharedlib_python_list(this);
> +    /* the list will decref all of its elements */
> +    Py_DECREF(this->threads);
> +    Py_DECREF(this->libs);
>      this->stacktrace->threads = NULL;
>      this->stacktrace->libs = NULL;
>      sr_gdb_stacktrace_free(this->stacktrace);
> @@ -624,11 +575,8 @@ sr_py_gdb_stacktrace_normalize(PyObject *self, PyObject *args)
>      /* need to rebuild python list manually */
>      struct sr_gdb_stacktrace *tmp = sr_gdb_stacktrace_dup(this->stacktrace);
>      sr_normalize_gdb_stacktrace(tmp);
> -    if (stacktrace_free_thread_python_list(this) < 0)
> -    {
> -        sr_gdb_stacktrace_free(tmp);
> -        return NULL;
> -    }
> +    /* the list will decref all of its elements */
> +    Py_DECREF(this->threads);
>  
>      this->stacktrace->threads = tmp->threads;
>      tmp->threads = NULL;
> diff --git a/python/py_gdb_thread.c b/python/py_gdb_thread.c
> index f9fde3c..4891375 100644
> --- a/python/py_gdb_thread.c
> +++ b/python/py_gdb_thread.c
> @@ -161,7 +161,8 @@ void
>  sr_py_gdb_thread_free(PyObject *object)
>  {
>      struct sr_py_gdb_thread *this = (struct sr_py_gdb_thread *)object;
> -    frames_free_python_list((struct sr_py_base_thread*) this);
> +    /* the list will decref all of its elements */
> +    Py_DECREF(this->frames);
>      this->thread->frames = NULL;
>      sr_gdb_thread_free(this->thread);
>      PyObject_Del(object);
> diff --git a/python/py_java_stacktrace.c b/python/py_java_stacktrace.c
> index 7a9477d..2686912 100644
> --- a/python/py_java_stacktrace.c
> +++ b/python/py_java_stacktrace.c
> @@ -135,7 +135,8 @@ void
>  sr_py_java_stacktrace_free(PyObject *object)
>  {
>      struct sr_py_java_stacktrace *this = (struct sr_py_java_stacktrace*)object;
> -    threads_free_python_list((struct sr_py_multi_stacktrace *)this);
> +    /* the list will decref all of its elements */
> +    Py_DECREF(this->threads);
>      this->stacktrace->threads = NULL;
>      sr_java_stacktrace_free(this->stacktrace);
>      PyObject_Del(object);
> diff --git a/python/py_java_thread.c b/python/py_java_thread.c
> index 3c8a77b..7869fef 100644
> --- a/python/py_java_thread.c
> +++ b/python/py_java_thread.c
> @@ -156,7 +156,8 @@ void
>  sr_py_java_thread_free(PyObject *object)
>  {
>      struct sr_py_java_thread *this = (struct sr_py_java_thread *)object;
> -    frames_free_python_list((struct sr_py_base_thread *)this);
> +    /* the list will decref all of its elements */
> +    Py_DECREF(this->frames);
>      this->thread->frames = NULL;
>      sr_java_thread_free(this->thread);
>      PyObject_Del(object);
> diff --git a/python/py_koops_stacktrace.c b/python/py_koops_stacktrace.c
> index c748721..0057cb1 100644
> --- a/python/py_koops_stacktrace.c
> +++ b/python/py_koops_stacktrace.c
> @@ -194,7 +194,8 @@ void
>  sr_py_koops_stacktrace_free(PyObject *object)
>  {
>      struct sr_py_koops_stacktrace *this = (struct sr_py_koops_stacktrace*)object;
> -    frames_free_python_list((struct sr_py_base_thread *)this);
> +    /* the list will decref all of its elements */
> +    Py_DECREF(this->frames);
>      this->stacktrace->frames = NULL;
>      sr_koops_stacktrace_free(this->stacktrace);
>      PyObject_Del(object);
> @@ -252,11 +253,8 @@ sr_py_koops_stacktrace_normalize(PyObject *self, PyObject *args)
>      /* need to rebuild python list manually */
>      struct sr_koops_stacktrace *tmp = sr_koops_stacktrace_dup(this->stacktrace);
>      sr_normalize_koops_stacktrace(tmp);
> -    if (frames_free_python_list((struct sr_py_base_thread *)this) < 0)
> -    {
> -        sr_koops_stacktrace_free(tmp);
> -        return NULL;
> -    }
> +    /* the list will decref all of its elements */
> +    Py_DECREF(this->frames);
>  
>      this->stacktrace->frames = tmp->frames;
>      tmp->frames = NULL;
> diff --git a/python/py_python_stacktrace.c b/python/py_python_stacktrace.c
> index 0f16b07..1b45e64 100644
> --- a/python/py_python_stacktrace.c
> +++ b/python/py_python_stacktrace.c
> @@ -151,7 +151,8 @@ void
>  sr_py_python_stacktrace_free(PyObject *object)
>  {
>      struct sr_py_python_stacktrace *this = (struct sr_py_python_stacktrace*)object;
> -    frames_free_python_list((struct sr_py_base_thread *)this);
> +    /* the list will decref all of its elements */
> +    Py_DECREF(this->frames);
>      this->stacktrace->frames = NULL;
>      sr_python_stacktrace_free(this->stacktrace);
>      PyObject_Del(object);




More information about the Crash-catcher mailing list