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

[Paul Ganssle]

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

> Adding a static assertion about the signedness of uint8_t looks meaningless to me.

My proposal is to add a static assertion that `day` is an unsigned type. In the code it would look something like it does in the backports.zoneinfo code (https://github.com/pganssle/zoneinfo/blob/07ec80ad5dc7e7e4b4f861ddbb61a9b71e9f27c7/lib/zoneinfo_module.c#L1247-L1255):


```
    // The actual bounds of day are (day >= 0 && day <= 6), but since day is an
    // unsigned variable, day >= 0 is always true. To ensure that a bug is not
    // introduced in the event that someone changes day to a signed type, we
    // will assert that it is an unsigned type.
    Py_ASSERT_VAR_UNSIGNED(day);
    if (day > 6) {
        PyErr_Format(PyExc_ValueError, "Day must be in [0, 6]");
        return -1;
    }
```

> I propose to change types of function parameters and local variables. In any case the constructor checks the range of parameters, so there is no problem with packing them into compact structure.

> This will help to avoid errors of implicit conversion between different integer types. Also it can help to avoid code duplication in parsing integers of different size and signedness.

This is not entirely unreasonable in my opinion, though in this case everything is determined by the POSIX standard and an RFC, so it is unlikely that we'll ever see anything outside of 8 bit integers for these things. If you'd like to refactor the parsing code to use signed integers unconditionally and have them converted as part of the constructor that seems reasonable.

----------

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