On 16 February 2014 02:52, Serhiy Storchaka <storch...@gmail.com> wrote:
> There are objections to these patches. Raymond against backporting the patch
> unless some known bugs are being fixed [1]. But it fixes at least one bug
> that caused a crash. And I suspect that there may be other bugs, just we
> still have no reproducers. Even if we don't know how to reproduce the bug,
> the current code looks obviously wrong. Also porting the patch will make the
> sync easier. Note that the patches were automatically generated, which
> reduces the possibility of errors. I just slightly corrected formatting,
> remove unused variables and fixed one error.
It's also likely than many of these crashes could only be reproduced
through incorrect usage of the C API.
Py_CLEAR/Py_XCLEAR was relatively simple, as there's only one pointer involved.
As Martin noted, Py_REPLACE is more complex, as there's the question
of whether or not the refcount for the RHS gets incremented.
Rather than using a completely different name, perhaps we can leverage
"Py_CLEAR" to make it more obvious that this operation is "like
Py_CLEAR and for the same reason, just setting the pointer to
something other than NULL".
For example:
Py_CLEAR_AND_SET
Py_XCLEAR_AND_SET
Such that Py_CLEAR and Py_XCLEAR are equivalent to:
Py_CLEAR_AND_SET(target, NULL);
Py_XCLEAR_AND_SET(target, NULL);
(Note that existing occurrences of SET in C API macros like
PyList_SET_ITEM and PyTuple_SET_ITEM also assume that any required
reference count adjustments are handled externally).
While the name does suggest the macro will expand to:
Py_CLEAR(target);
target = value;
which isn't quite accurate, it's close enough that people should still
be able to understand what the operation does. Explicitly pointing out
in that docs that Py_CLEAR(target) is functionally equivalent to
Py_CLEAR_AND_SET(target, NULL) should help correct the
misapprehension.
On the question of updating 3.4+ only vs also updating 3.3, I'm
inclined towards fixing it systematically in all currently supported
branches, as it's a low risk mechanical change. On the other hand, we
still have other crash bugs with known reproducers (in
Lib/test/crashers), so I'm also OK with leaving 3.3 alone. (Assuming
we can agree on a name, I definitely think it's worth asking Larry for
permission to make the late C API addition for 3.4, though)
Regards,
Nick.
--
Nick Coghlan | ncogh...@gmail.com | Brisbane, Australia
_______________________________________________
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
