Author xiang.zhang
Recipients rhettinger, serhiy.storchaka, xiang.zhang
Date 2016-08-08.03:05:06
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1470625507.51.0.39850156958.issue27703@psf.upfronthosting.co.za>
In-reply-to
Content
Thanks for your reply Raymond. You always provide thoughtful feedbacks, though it makes me somewhat more nervous. ;) (I am nervous about making noise here and waste others' time.)

> Have you checked all the possibly code paths to be sure?

Yes. I did check all the code paths. I don't want to be a noise maker so I did check it carefully.

>  "Seems to be unnecessary" is insufficient reason for undoing Guido's code that has stood since 1997.

I use the word "seems" because even if I think my opinion is right, I am only 99% sure. Just as you said, the code is legacy and changing it has to be careful. I can miss something and be wrong. But now I think I am right.

> Looking at the snarl of possible code paths, I not finding it obvious why some of the paths require Py_XDECREF and others don't.

When we reach the two Py_XDECREFs, can type and value be NULL?

> then the patch needs to include assertions at key checkpoints (i.e. after a given assignment to "type" or "value" can we reliably assert the value is non-null) and/or comments showing what the reasoning is

Sorry, I didn't realize that. This is definitely good practice I know. But this is not documented and not a must then so I didn't do that. But I'll remember this in later patches.
History
Date User Action Args
2016-08-08 03:05:07xiang.zhangsetrecipients: + xiang.zhang, rhettinger, serhiy.storchaka
2016-08-08 03:05:07xiang.zhangsetmessageid: <1470625507.51.0.39850156958.issue27703@psf.upfronthosting.co.za>
2016-08-08 03:05:07xiang.zhanglinkissue27703 messages
2016-08-08 03:05:06xiang.zhangcreate