I hadn't noticed this macro defined in Include/object.h before: /* Helper for passing objects to printf and the like */ #define PyObject_REPR(obj) PyString_AS_STRING(PyObject_Repr(obj)) I may not be right, but I don't see how this can't help but not free the intermediate PyString object. It doesn't seem to be used, except for in situations where Python is not going to continue working much longer anyway (specifically, in compile.c and ceval.c.) I could not be right about *that*, too, though ;-) It strikes me that it should not be used, or maybe renamed to _PyObject_REPR. It's wasn't added in 2.5 or 2.4, though, so it's not particularly new and I can't guarantee that it's not used in any third party code. But, then again, anyone using it isn't free of leaks. Should removing or renaming it be done in 2.5 or in Py3K? Triple-negative'ly y'rs, -- Thomas Wouters <thomas@python.org> Hi! I'm a .signature virus! copy me into your .signature file to help me spread!
I may not be right, but I don't see how this can't help but not free the intermediate PyString object.
Good catch.
It doesn't seem to be used, except for in situations where Python is not going to continue working much longer anyway (specifically, in compile.c and ceval.c.)
Those internal uses ought to be replaced with better code.
It strikes me that it should not be used, or maybe renamed to _PyObject_REPR. It's wasn't added in 2.5 or 2.4, though, so it's not particularly new and I can't guarantee that it's not used in any third party code. But, then again, anyone using it isn't free of leaks. Should removing or renaming it be done in 2.5 or in Py3K?
Since it is intrinsically buggy, I would support removal in Py2.5 Raymond
On 4/11/06, Raymond Hettinger <raymond.hettinger@verizon.net> wrote:
It strikes me that it should not be used, or maybe renamed to _PyObject_REPR. Should removing or renaming it be done in 2.5 or in Py3K?
Since it is intrinsically buggy, I would support removal in Py2.5
+1 on removal. Google only turned up a handleful of uses that I saw. n
It's intended as an internal debugging API. I find it very convenient for adding with a printf() here and there, which is how it got added long ago. It should really have a comment mentioning that it leaks the repr object, and starting with an _ wouldn't be bad either. Jeremy On 4/11/06, Neal Norwitz <nnorwitz@gmail.com> wrote:
On 4/11/06, Raymond Hettinger <raymond.hettinger@verizon.net> wrote:
It strikes me that it should not be used, or maybe renamed to _PyObject_REPR. Should removing or renaming it be done in 2.5 or in Py3K?
Since it is intrinsically buggy, I would support removal in Py2.5
+1 on removal. Google only turned up a handleful of uses that I saw.
n _______________________________________________ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/jeremy%40alum.mit.edu
Ok, then how about prefixing with _, adding a comment saying in big, bold letters: FOR DEBUGGING PURPOSES ONLY, THIS LEAKS, and only defining in a debug build? n -- On 4/11/06, Jeremy Hylton <jeremy@alum.mit.edu> wrote:
It's intended as an internal debugging API. I find it very convenient for adding with a printf() here and there, which is how it got added long ago. It should really have a comment mentioning that it leaks the repr object, and starting with an _ wouldn't be bad either.
Jeremy
On 4/11/06, Neal Norwitz <nnorwitz@gmail.com> wrote:
On 4/11/06, Raymond Hettinger <raymond.hettinger@verizon.net> wrote:
It strikes me that it should not be used, or maybe renamed to _PyObject_REPR. Should removing or renaming it be done in 2.5 or in Py3K?
Since it is intrinsically buggy, I would support removal in Py2.5
+1 on removal. Google only turned up a handleful of uses that I saw.
n _______________________________________________ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/jeremy%40alum.mit.edu
If PyObject_REPR changes or gets renamed in Py2.5, I suggest modifying the implementation so that it returns a newly allocated C pointer rather one embedded in an inaccessible (unfreeable) PyStringObject. Roughly: r = PyObject_Repr(o); if (r == NULL) return NULL; s1 = PyObject_AS_STRING(r); s2 = strcpy(s1); Py_DECREF(r); return s2; The benefits are: * it won't throw-off leak checking (no Python objects get leaked) * the leak is slightly smaller (only the allocated string) * if the caller cares about memory, they have the option of freeing the returned pointer * error-checking is still possible. Neal Norwitz wrote:
Ok, then how about prefixing with _, adding a comment saying in big, bold letters: FOR DEBUGGING PURPOSES ONLY, THIS LEAKS, and only defining in a debug build?
n -- On 4/11/06, Jeremy Hylton <jeremy@alum.mit.edu> wrote:
It's intended as an internal debugging API. I find it very convenient for adding with a printf() here and there, which is how it got added long ago. It should really have a comment mentioning that it leaks the repr object, and starting with an _ wouldn't be bad either.
Jeremy
On 4/11/06, Neal Norwitz <nnorwitz@gmail.com> wrote:
On 4/11/06, Raymond Hettinger <raymond.hettinger@verizon.net> wrote:
It strikes me that it should not be used, or maybe renamed to _PyObject_REPR. Should removing or renaming it be done in 2.5 or in Py3K?
Since it is intrinsically buggy, I would support removal in Py2.5
+1 on removal. Google only turned up a handleful of uses that I saw.
n _______________________________________________ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/jeremy%40alum.mit.edu
Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/rhettinger%40ewtllc.com
Jeremy Hylton wrote:
It's intended as an internal debugging API. I find it very convenient for adding with a printf() here and there, which is how it got added long ago. It should really have a comment mentioning that it leaks the repr object, and starting with an _ wouldn't be bad either.
I'm still in favour of removing it. If you need it in debugging code, writing PyObject_AsString(PyObject_Repr(o)) doesn't seem too hard, IMO. If you write it so often that it hurts, you can put it at the top of your file (and perhaps also give it a shorter name, such as P(o)). Nobody but you seemed to know about its existence, and, given that it is flawed, neither its definition nor its use belong into Python (IMO, of course). Regards, Martin
participants (6)
-
"Martin v. Löwis" -
Jeremy Hylton -
Neal Norwitz -
Raymond Hettinger -
Raymond Hettinger -
Thomas Wouters