[issue1621] Do not assume signed integer overflow behavior

[Mark Dickinson]

Mark Dickinson <dickinsm at gmail.com> added the comment:

> [and then perform the multiplication unsigned, to silence the
> warning - right?]

That wasn't actually what I was thinking:  I was proposing to rewrite it 
as:

	if (Py_SIZE(a) > 0 && n > PY_SSIZE_T_MAX/Py_SIZE(a)) {
		PyErr_SetString(PyExc_OverflowError,
			"repeated bytes are too long");
		return NULL;
	}
	size = Py_SIZE(a) * n;

The multiplication should be safe from overflow, and I don't get
any warning at all either with this rewrite (using -O3 -Wall -Wextra -
Wsigned-overflow=5) or from the original code, so there's nothing to 
silence.

> I think there is a second solution: perform the multiplication
> unsigned in the first place.

That would work too.  I find the above code clearer, though.  It's not 
immediately obvious to me that the current overflow condition actually 
works, even assuming wraparound on overflow; I find myself having to 
think about the mathematics every time.

In general, it seems to me that the set of places reported by -Wsigned-
overflow is a poor match for the set of places that need to be fixed.  -
Wsigned-overflow only gives a warning when that particular version of 
gcc, with those particular flags, happens to make use of the no-overflow 
assumption for some particular optimization.  Certainly each of the 
places reported by -Wsigned-overflow should be investigated, but I don't 
believe it's worth 'fixing' correct code just to get rid of warnings 
from this particular warning option.

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue1621>
_______________________________________