Comment 2 for bug 457979

Revision history for this message
Matt Nordhoff (mnordhoff) wrote : Re: [Bug 457979] [NEW] StaticTuple_New handles size < 0 and size > 255 differently

John A Meinel wrote:
> Matt Nordhoff wrote:
>> Public bug reported:
>
>> Lookie:
>
>> if (size < 0) {
>> PyErr_BadInternalCall();
>> return NULL;
>> }
>
>> if (size < 0 || size > 255) {
>> /* Too big or too small */
>> PyErr_SetString(PyExc_ValueError, "StaticTuple(...)"
>> " takes from 0 to 255 items");
>> return NULL;
>> }
>
>> Not only is this redundant, but PyErr_BadInternalCall() throws a
>> TypeError, meaning two different exception classes are used.
>
>> ISTM StaticTuple_New should throw a ValueError, since you do
>> StaticTuple_New(length), and StaticTuple_new_constructor should throw a
>> TypeError, since you do StaticTuple(foo, bar, baz) (currently it lets
>> StaticTuple_New handle throw the error).
>
>> FYI, the Python version of StaticTuple throws a ValueError when it's
>> either <0 or >255. IMO, that should be a TypeError, same as
>> StaticTuple_new_constructor.
>
>> ** Affects: bzr
>> Importance: Undecided
>> Status: New
>
>> ** Description changed:
>
>> Lookie:
>
>> - if (size < 0) {
>> - PyErr_BadInternalCall();
>> - return NULL;
>> - }
>> + if (size < 0) {
>> + PyErr_BadInternalCall();
>> + return NULL;
>> + }
>
>> - if (size < 0 || size > 255) {
>> - /* Too big or too small */
>> - PyErr_SetString(PyExc_ValueError, "StaticTuple(...)"
>> - " takes from 0 to 255 items");
>> - return NULL;
>> - }
>> + if (size < 0 || size > 255) {
>> + /* Too big or too small */
>> + PyErr_SetString(PyExc_ValueError, "StaticTuple(...)"
>> + " takes from 0 to 255 items");
>> + return NULL;
>> + }
>
>> Not only is this redundant, but PyErr_BadInternalCall() throws a
>> TypeError, meaning two different exception classes are used.
>
> Actually, PyErr_BadInternalCall raises a "SystemError":
> void
> _PyErr_BadInternalCall(char *filename, int lineno)
> {
> PyErr_Format(PyExc_SystemError,
>
> ...

Oh, huh. Thanks for nothing,
<http://docs.python.org/c-api/exceptions.html#PyErr_BadInternalCall>.

> My specific point was that passing size < 0 is a sign that something is
> very wrong. Which is different from just passing a value that is too large.

Hm, that makes sense.

> The redundant "<0" is probably because I used to check for size < 1, and
> just didn't remove that check when I started allowing size 0 StaticTuples.

Oh, okay.

>> ISTM StaticTuple_New should throw a ValueError, since you do
>> StaticTuple_New(length), and StaticTuple_new_constructor should throw a
>> - TypeError, since you do StaticTuple(foo, bar, baz).
>> + TypeError, since you do StaticTuple(foo, bar, baz) (currently it lets
>> + StaticTuple_New handle throw the error).
>
>> FYI, the Python version of StaticTuple throws a ValueError when it's
>> either <0 or >255. IMO, that should be a TypeError, same as
>> StaticTuple_new_constructor.
>
>
> I'm happy to have them the same. Regardless of what we make this, it
> isn't something you should ever be catching, so the specific exception
> isn't particularly meaningful.
>
> It is arguable that "TypeError" should be reserved for when the *type*
> of the argument passed is invalid. (Such as passing a dict rather than a
> string/unicode/etc.)
>
> Everything else would be considered a ValueError. (Having a tuple that
> holds only strings but is 266 entries long is closer to a ValueError
> than a TypeError.)

While it's a little odd (IMO), Python likes to use TypeError when you
pass the wrong number of arguments to a function:

>>> def f(): pass
...
>>> f(1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: f() takes no arguments (1 given)

If we imagine StaticTuple as being defined like "def __new__(cls,
arg1=None, arg2=None, arg3=None, ..., arg255=None)", and someone passed
256 arguments, Python would raise a TypeError.

> John
> =:->
--