Message87708
> [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. |
|
Date |
User |
Action |
Args |
2009-05-13 20:58:29 | mark.dickinson | set | recipients:
+ mark.dickinson, gvanrossum, loewis, nnorwitz, gregory.p.smith, pitrou, vstinner, christian.heimes, alexandre.vassalotti, donmez, matejcik |
2009-05-13 20:58:29 | mark.dickinson | set | messageid: <1242248309.07.0.900383605185.issue1621@psf.upfronthosting.co.za> |
2009-05-13 20:58:27 | mark.dickinson | link | issue1621 messages |
2009-05-13 20:58:26 | mark.dickinson | create | |
|