assert for NULL in Py_INCREF Py_DECREF

[illume]

BPO	1517947
Nosy	@brettcannon, @birkenfeld

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2007-03-13.21:55:06.000>
created_at = <Date 2006-07-06.06:43:16.000>
labels = []
title = 'assert for NULL in Py_INCREF Py_DECREF'
updated_at = <Date 2007-03-13.21:55:06.000>
user = 'https://bugs.python.org/illume'

bugs.python.org fields:

activity = <Date 2007-03-13.21:55:06.000>
actor = 'georg.brandl'
assignee = 'none'
closed = True
closed_date = None
closer = None
components = ['None']
creation = <Date 2006-07-06.06:43:16.000>
creator = 'illume'
dependencies = []
files = []
hgrepos = []
issue_num = 1517947
keywords = ['patch']
message_count = 3.0
messages = ['50621', '50622', '50623']
nosy_count = 3.0
nosy_names = ['brett.cannon', 'georg.brandl', 'illume']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue1517947'
versions = []

[illume]

Since Py_INCREF and Py_DECREF should not be able to
take NULLs they should do an assert check for this.

This would have caught at least one bug earlier in
cPickle.loads()
http://sourceforge.net/tracker/index.php?func=detail&aid=1512695&group_id=5470&atid=105470

It will also help other extension module authors find
this error in their code more easily.

Include/object.h
#define Py_INCREF(op) (			        \
        (assert((op) != NULL)) ,  	        \
        _Py_INC_REFTOTAL  _Py_REF_DEBUG_COMMA	\
        (op)->ob_refcnt++)                      


#define Py_DECREF(op)					\
	if ((assert((op) != NULL)) , _Py_DEC_REFTOTAL 
_Py_REF_DEBUG_COMMA	\
	    --(op)->ob_refcnt != 0)			\
		_Py_CHECK_REFCNT(op)			\
	else						\
		_Py_Dealloc((PyObject *)(op))

[brettcannon]

Logged In: YES
user_id=357491

I don't think that the cPickle bug would have been any
sooner. The segfault happens just as early as an assertion
would have since both going to access bad memory.

Regardless, the adding of the assert() in Py_DECREF might
warrant using while{} do(0) to simplify the code.

[birkenfeld]

Quoting Martin v. Löwis:
"""
I think it's pointless. If they ever be NULL, the INCRE/DECREF
will crash right away, anyway, so you'll certainly notice.
As you will have a debug build when the assertion triggers,
you can just as well fine the location of the crash quickly
in the debugger (e.g. using the core file on systems that
support that, or the JIT-debugger on other systems).
"""

Rejecting on that basis.