This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author mark.dickinson
Recipients alexandre.vassalotti, christian.heimes, donmez, gregory.p.smith, gvanrossum, loewis, mark.dickinson, matejcik, nnorwitz, pitrou, vstinner
Date 2009-05-13.15:53:04
SpamBayes Score 2.168107e-07
Marked as misclassified No
Message-id <1242229987.52.0.545866450685.issue1621@psf.upfronthosting.co.za>
In-reply-to
Content
A few comments: (1) I agree that signed overflows should be avoided where possible. (2) I think some of the changes in the latest patch (fix-overflows-final.patch) are unnecessary, and decrease readability a bit. An example is the following chunk for the x_divrem function in Objects/longobject.c. @@ -1799,7 +1799,7 @@ x_divrem(PyLongObject *v1, PyLongObject *w1, PyLongObject **prem)	k = size_v - size_w;	a = _PyLong_New(k + 1); -	for (j = size_v; a != NULL && k >= 0; --j, --k) { +	for (j = size_v; a != NULL && k >= 0; j = (unsigned)j - 1 , k = (unsigned)k - 1) {	digit vj = (j >= size_v) ? 0 : v->ob_digit[j];	twodigits q;	stwodigits carry = 0; In this case it's easy to see from the code that j and k will always be nonnegative, so replacing --j with j = (unsigned)j - 1 is unnecessary. (This chunk no longer applies for 2.7 and 3.1, btw, since x_divrem got rewritten recently.) Similar comments apply to the change: -	min_gallop -= min_gallop > 1; +	if (min_gallop > 1) min_gallop = (unsigned)min_gallop - 1; in Objects/listobject.c. Here it's even clearer that the cast is unnecessary. I assume these changes were made to silence warnings from -Wstrict-overflow, but I don't think that should be a goal: I'd suggest only making changes where there's a genuine possibility of overflow (even if it's a remote one), and leaving the code unchanged if it's reasonably easy to see that overflow is impossible. (3) unicode_hash also needs fixing, as do the lookup algorithms for set and dict. Both use overflowing arithmetic on signed types as a matter of course. Probably a good few of the hash algorithms for the various object types in Objects/ are suspect.
History
Date User Action Args
2009-05-13 15:53:07mark.dickinsonsetrecipients: + mark.dickinson, gvanrossum, loewis, nnorwitz, gregory.p.smith, pitrou, vstinner, christian.heimes, alexandre.vassalotti, donmez, matejcik
2009-05-13 15:53:07mark.dickinsonsetmessageid: <1242229987.52.0.545866450685.issue1621@psf.upfronthosting.co.za>
2009-05-13 15:53:06mark.dickinsonlinkissue1621 messages
2009-05-13 15:53:04mark.dickinsoncreate