On 04/ 2/10 05:34 PM, [email protected] wrote:
On Fri, Apr 02, 2010 at 05:14:19PM -0500, Shawn Walker wrote:
On 04/ 2/10 04:46 PM, [email protected] wrote:
On Sun, Mar 28, 2010 at 08:31:16PM -0500, Shawn Walker wrote:
webrev:
http://cr.opensolaris.org/~swalker/pkg-slots/
_actions.c:
- The reference counting here needs to be changed. The typical
calling convention for Python C modules is that the callee borrows
the references from the caller, unless explicitly documented
otherwise.
Yes, but I was trying to avoid adding an extra function or in-lining
the logic for getting the cached attribute from the dictionary.
That's why the inc/dec logic was added to add_to_attrs -- it seemed
logical to do it one place. The python documents seem to be
inconsistent about recommendations for when it's safe to borrow
references, and when it's not.
I don't think what I'm suggesting will cause you to need to add an extra
function, or inline anything else. I'm just asking that you move the
INCREF and DECREF calls, since there's no documentation about the
calling convention. IMO, the current way that the references are
handled is pretty confusing and my hope is that what I'm suggesting
simplifies the understanding of this code for others who read it.
Sorry, that wasn't what I was implying. I don't mind making the changes
at all, especially since I think it's better than what was there even
before my changes.
...
c. Change add_to_attrs() to only Py_DECREF references that it has
INCREF'd or taken as the result of a function call.
There's a further caveat when assigning memo_entry to key on line
48: you now have key with a reference that's been added by this
function. I believe that to solve the problem with uneven reference
counts, you need to Py_INCREF(key) before line 43 so that subsequent
DECREF's don't get the reference count confused.
Yes, that was intentional. I don't quite follow the uneven
reference counts bit though. Although looking at this, I can see
other problems, but nothing that points to uneven reference counts.
Can you expound a bit?
Sorry, what I meant is that if you make the changes so that _fromstr()
is supposed to DECREF key, then you need to INCREF key on line 43. This
is to account for the cases where key is not reassigned to memo_entry.
Ah, yes. I understand now. The PyDict_SetItem does INCREF it, but if
_fromstr() does a decref now instead, the extra is needed.
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
