[issue42660] _zoneinfo.c incorrectly checks bounds of `day` variable in calenderrule_new

[Paul Ganssle]

Paul Ganssle <p.ganssle at gmail.com> added the comment:

There are at least two issues with this:

1. This is a constructor for a struct, and the struct would really unnecessarily balloon in size if we switched everything over to be 32-bit integers (which I think is your suggestion?). This is not a major concern because in typical operation there are not likely to be more than 500 of these structs in existence at any one time (due to the cache), but it's still a minor annoyance.

I would guess that accepting `int` in the constructor function and converting it to uint8_t as part of building the struct would be maybe neutral to negative, since it involves casting a large signed type to a small unsigned one. This could cause more problems with overflow, not less.

2. In the current implementation `day` is unsigned by its nature — it represents a value indexed at 0 for which negative indices have no meaning. If we leave `day` as a uint8_t, then that tells the compiler at least something about the valid range of the value. By turning `day` (and presumably the other elements of this struct) into a less-suitable type, we're depriving ourselves of possibly *useful* compiler warnings.

Barring a solution where we have a simple and robust way to suppress warnings, I think the next-best solution is to add a static assertion about the signedness of the type for this particular case. I'd be happy to write it in a relatively general way so that it can be re-used elsewhere in CPython for the same purpose.

----------

_______________________________________
Python tracker <report at bugs.python.org>
<https://bugs.python.org/issue42660>
_______________________________________