#17 C module memory leak

closed-invalid
nobody
None
5
2001-06-07
2001-05-31
No

PY-XMLRPC 0.8.5

File "rpcSource.c", routine "pyRpcSourceSetDesc":

You have:

...
unless (PyArg_ParseTuple(args, "s", &desc))
return NULL;

if (srcp->desc)
free(srcp->desc);
srcp->desc = alloc(strlen(desc));
if (srcp->desc == NULL)
return NULL;
strcpy(srcp->desc, desc);
...

You are duplicating the string (beware about the strcpy
off by one, comented in other bug report).

"PyArg_ParseTuple" already gives you a appropiate
string to keep. I suggest the following change:

...
unless (PyArg_ParseTuple(args, "s", &desc))
return NULL;

if (srcp->desc)
free(srcp->desc);
srcp->desc = desc
...

There are similar situations in other routines, but I
can´t check them all right now.

Discussion

  • Shilad Sen
    Shilad Sen
    2001-05-31

    • status: open --> open-rejected
     
  • Shilad Sen
    Shilad Sen
    2001-05-31

    Logged In: YES
    user_id=184164

    This is already freed in the dealloc function for rpcSource:

    void
    rpcSourceDealloc(rpcSource *srcp)
    {
    ....blah...
    if (srcp->desc) {
    free(srcp->desc);
    srcp->desc = NULL;
    }
    ,,,,blah....
    }

     
  • Logged In: YES
    user_id=97460

    You are missing the point.

    The memory you leak is the string that "PyArg_ParseTuple"
    gives you.

    "PyArg_ParseTuple" gives you a string. You copied it. Then,
    later, you free it. Okay.

    But you DONT free the original string that
    "PyArg_ParseTuple" gives you.

     
  • Shilad Sen
    Shilad Sen
    2001-05-31

    Logged In: YES
    user_id=184164

    As far as I know, when you get a string from
    PyArg_ParseTuple it is NOT a copy. It is simply the char *
    pointed to the python object. If you freed the char *, the
    python object would be pointing to invalid memory space.
    There is no need to free it, and if you want to hold on to
    it you must make your own copy.

    Check out the source for python in getargs.c, function
    convertsimple1():

    case 's': /* string */
    ...blah...
    if (PyString_Check(arg))
    *p =
    PyString_AsString(arg);
    else
    return "string";
    if ((int)strlen(*p) !=
    PyString_Size(arg))
    return "string without
    null bytes";
    }
    break;
    }

    It certainly doesn't look like it's making a copy. Am I
    missing something?

     
  • Shilad Sen
    Shilad Sen
    2001-06-07

    Logged In: YES
    user_id=184164

    This was tested and does not memory leak.

     
  • Shilad Sen
    Shilad Sen
    2001-06-07

    • status: open-rejected --> closed-invalid