this patch implements additional overflow checks for lists of arguments and support for other structures that can have more than INT_MAX elements in Py2.5
This looks ok, but I don't think the INT_MAX checks are necessary. I'm looking at the .h files and PEP 353, and I don't think it is possible to write something that supports the abstract PySequence interface and can hold more than PY_SSIZE_T_MAX elements; it simply won't fit in the type.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I'm fairly certain you're wrong. _mysql.c has the prescribed cleverness at the top:
if PY_VERSION_HEX < 0x02050000 && !defined(PY_SSIZE_T_MIN)
typedef int Py_ssize_t;
define PY_SSIZE_T_MAX INT_MAX
define PY_SSIZE_T_MIN INT_MIN
endif
This means that if we're in the pre-PEP 353 era, create a typedef named Py_ssize_t and two macros (PY_SSIZE_T_MAX and _MIN) to help module authors write compatible code. I think you may be getting turned around by the way typedef and #define take their arguments in reverse order.
If you're compiling pre-2.5 on 64-bit, Py_ssize_t == int and PY_SSIZE_T_MAX == INT_MAX .
If you have a sequence that can hold more than PY_SSIZE_T_MAX elements, you're doing something wrong. It should be throwing and OverflowError.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
what you say is all true. however, i'm handling post-2.5 pythons, specifically the situations where sizeof(Py_ssize_t) > sizeof(int), and so the index actually can be bigger than INT_MAX (but obviously not bigger than PY_SSIZE_T_MAX).
and since the whole thing uses int's as indices, and i didn't really go into rewriting all that, i'm led to believe that checking for int overflow (which still can occur, as opposed to Py_ssize_t overflow, about which we don't really need to worry) is the sane thing to do.
i will admit that i don't understand the inner workings of _mysql.c enough to be sure that there's a chance of overflow permitted by the module logic - plus, there may have been changes to the code since i posted the patch, and i'm not at work now so i can't easily check if it still applies - but the sane thing to do in the preexisting-to-py_ssize_t-aware interface is guard the preexisting code against insane (i.e. >INT_MAX) values that may come out of the py_ssize_t-aware part
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Rev 573 applies the fixed declaration to trunk, minus the overflow checks. The internals of Python's sequence types check to ensure overflows don't occur as the sequence is modified (throwing a MemoryError). So once we get one passed to us here, there isn't a need for checks.
With this being a "correctness" patch, not manifesting a bug, and 1.2.3 being stabilized for release, I think this can stay in trunk.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
patch for version 1.2.2
This looks ok, but I don't think the INT_MAX checks are necessary. I'm looking at the .h files and PEP 353, and I don't think it is possible to write something that supports the abstract PySequence interface and can hold more than PY_SSIZE_T_MAX elements; it simply won't fit in the type.
true, but INT_MAX < PY_SSIZE_T_MAX on many compilers, that's why the checks are there
(on 64bit environments, that is)
I'm fairly certain you're wrong. _mysql.c has the prescribed cleverness at the top:
if PY_VERSION_HEX < 0x02050000 && !defined(PY_SSIZE_T_MIN)
typedef int Py_ssize_t;
define PY_SSIZE_T_MAX INT_MAX
define PY_SSIZE_T_MIN INT_MIN
endif
This means that if we're in the pre-PEP 353 era, create a typedef named Py_ssize_t and two macros (PY_SSIZE_T_MAX and _MIN) to help module authors write compatible code. I think you may be getting turned around by the way typedef and #define take their arguments in reverse order.
If you're compiling pre-2.5 on 64-bit, Py_ssize_t == int and PY_SSIZE_T_MAX == INT_MAX .
If you have a sequence that can hold more than PY_SSIZE_T_MAX elements, you're doing something wrong. It should be throwing and OverflowError.
what you say is all true. however, i'm handling post-2.5 pythons, specifically the situations where sizeof(Py_ssize_t) > sizeof(int), and so the index actually can be bigger than INT_MAX (but obviously not bigger than PY_SSIZE_T_MAX).
and since the whole thing uses int's as indices, and i didn't really go into rewriting all that, i'm led to believe that checking for int overflow (which still can occur, as opposed to Py_ssize_t overflow, about which we don't really need to worry) is the sane thing to do.
i will admit that i don't understand the inner workings of _mysql.c enough to be sure that there's a chance of overflow permitted by the module logic - plus, there may have been changes to the code since i posted the patch, and i'm not at work now so i can't easily check if it still applies - but the sane thing to do in the preexisting-to-py_ssize_t-aware interface is guard the preexisting code against insane (i.e. >INT_MAX) values that may come out of the py_ssize_t-aware part
Rev 573 applies the fixed declaration to trunk, minus the overflow checks. The internals of Python's sequence types check to ensure overflows don't occur as the sequence is modified (throwing a MemoryError). So once we get one passed to us here, there isn't a need for checks.
With this being a "correctness" patch, not manifesting a bug, and 1.2.3 being stabilized for release, I think this can stay in trunk.