[Tim]
>> Phillip, when eyeballing gen_dealloc(), I didn't understand two things:
>>
>> 1. Why doesn't
>>
>> if (gen->gi_frame->f_stacktop!=NULL) {
>>
>> check first to be sure that gen->gi_frame != Py_None?
[Phillip]
> Apparently, it's because I'm an idiot, and because nobody else realized
> this during the initial review of the patch. :)
Then you were the bold adventurer, and the reviewers were the idiots ;-)
>> Is that impossible here for some reason?
> No, I goofed. It's amazing that this doesn't dump core whenever a
> generator exits. :(
Well, it's extremely likely that &((PyGenObject *)Py_None)->f_stacktop
is a legit part of the process address space, so that dereferencing is
non-problematic. We just read up nonsense then, test against it, and
the conditional
gen->ob_type->tp_del(self);
is harmless as gen_del() returns at once (because it does check for
Py_None right away -- we never try to treat gi_frame->f_stacktop _as_
a frame pointer here, we just check it against NULL). If we have to
have insane code, harmlessly insane is the best kind :--)
>> 2. It _looks_ like "gi_frame != NULL" is an (undocumented) invariant.
>> Right? If so,
>>
>> Py_XDECREF(gen->gi_frame);
>>
>> sends a confusing message (because of the "X", implying that NULL is OK).
>> Regardless, it would be good to add comments to genobject.h explaining
>> the possible values gi_frame can hold. For example, what does it mean
>> when gi_frame is Py_None? Can it ever be NULL?
> I think what happened is that at one point I thought I was going to set
> gi_frame=NULL when there's no active frame, in order to speed up
> reclamation of the frame. However, I think I then thought that it would
> break the operation of the generator's 'gi_frame' attribute, which is
> defined as T_OBJECT.
That shouldn't be a problem: when PyMember_GetOne() fetches a
T_OBJECT that's NULL, it returns (a properly incref'ed) Py_None
instead.
> Or else I thought that the tp_visit was screwed up in that case.
>
> So, it looks to me like what this *should* do is simply allow gi_frame to
> be NULL instead of Py_None, which would get rid of all the silly casting,
> and retroactively make the XDECREF correct. :)
That indeed sounds better to me, although you still don't get out of
writing gi_frame comments for genobject.h :-)
> Does gen_traverse() need to do anything special to visit a null
> pointer? Should it only conditionally visit it?
The body of gen_traverse() is best written:
Py_VISIT(gen->gi_frame);
return 0;
Py_VISIT is defined in objimpl.h, and takes care of NULLs and
exceptional returns.
BTW, someone looking for an easy task might enjoy rewriting other
tp_traverse slots to use Py_VISIT. We even have cases now (like
super_traverse) where modules define their own workalike
traverse-visit macros, which has become confusing since a standard
macro was defined for this purpose.
_______________________________________________
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/archive%40mail-archive.com
