Re: [libvirt] [PATCH] python: Don't free must-not-free variables

[Jiri Denemark <jdenemar redhat com>]

On Mon, May 23, 2011 at 14:47:29 +0200, Michal Privoznik wrote:
> py_str() function call PyString_AsString(). As written in documentation,
> the caller must not free the returned value, because it points to some
> internal structures.
> ---
>  python/libvirt-override.c |   15 ++++++---------
>  1 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/python/libvirt-override.c b/python/libvirt-override.c
> index a151e78..7998723 100644
> --- a/python/libvirt-override.c
> +++ b/python/libvirt-override.c
> @@ -35,6 +35,8 @@ extern void initcygvirtmod(void);
>  #define VIR_PY_INT_FAIL (libvirt_intWrap(-1))
>  #define VIR_PY_INT_SUCCESS (libvirt_intWrap(0))
>  
> +/* We don't want to free() returned value, as written in doc
> + * to PyString_AsString. */
>  static char *py_str(PyObject *obj)
>  {
>      PyObject *str = PyObject_Str(obj);
> @@ -2660,7 +2662,6 @@ libvirt_virEventAddHandleFunc  (int fd,
>          char *name = py_str(python_cb);
>          printf("%s: %s is not callable\n", __FUNCTION__,
>                 name ? name : "libvirt.eventInvokeHandleCallback");
> -        free(name);
>  #endif
>          goto cleanup;
>      }
> @@ -2805,7 +2806,6 @@ libvirt_virEventAddTimeoutFunc(int timeout,
>          char *name = py_str(python_cb);
>          printf("%s: %s is not callable\n", __FUNCTION__,
>                 name ? name : "libvirt.eventInvokeTimeoutCallback");
> -        free(name);
>  #endif
>          goto cleanup;
>      }
> @@ -2919,17 +2919,11 @@ libvirt_virEventRegisterImpl(ATTRIBUTE_UNUSED PyObject * self,
>  {
>      /* Unref the previously-registered impl (if any) */
>      Py_XDECREF(addHandleObj);
> -    free(addHandleName);
>      Py_XDECREF(updateHandleObj);
> -    free(updateHandleName);
>      Py_XDECREF(removeHandleObj);
> -    free(removeHandleName);
>      Py_XDECREF(addTimeoutObj);
> -    free(addTimeoutName);
>      Py_XDECREF(updateTimeoutObj);
> -    free(updateTimeoutName);
>      Py_XDECREF(removeTimeoutObj);
> -    free(removeTimeoutName);
>  
>      /* Parse and check arguments */
>      if (!PyArg_ParseTuple(args, (char *) "OOOOOO:virEventRegisterImpl",
> @@ -2944,7 +2938,10 @@ libvirt_virEventRegisterImpl(ATTRIBUTE_UNUSED PyObject * self,
>          !PyCallable_Check(removeTimeoutObj))
>          return VIR_PY_INT_FAIL;
>  
> -    /* Get argument string representations (for error reporting) */
> +    /* Get argument string representations (for error reporting).
> +     * py_str calls PyString_AsString() which returns pointer to:
> +     * 'internal buffer of string, not a copy.' and 'It must not be
> +     * deallocated' (pyhton c-api documentation) */

Given that py_str() is used in several places, I think this would actually fit
better in a comment for the py_str function itself.

>      addHandleName = py_str(addHandleObj);
>      updateHandleName = py_str(updateHandleObj);
>      removeHandleName = py_str(removeHandleObj);

ACK with that nit fixed.

Jirka