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,
...
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.
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.
>
> 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.)
John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Matt Nordhoff wrote: alCall( ); (PyExc_ ValueError, "StaticTuple(...)" alCall( ) throws a New(length) , and StaticTuple_ new_constructor should throw a new_constructor . alCall( ); alCall( ); (PyExc_ ValueError, "StaticTuple(...)" (PyExc_ ValueError, "StaticTuple(...)" alCall( ) throws a
> Public bug reported:
>
> Lookie:
>
> if (size < 0) {
> PyErr_BadIntern
> return NULL;
> }
>
> if (size < 0 || size > 255) {
> /* Too big or too small */
> PyErr_SetString
> " takes from 0 to 255 items");
> return NULL;
> }
>
> Not only is this redundant, but PyErr_BadIntern
> TypeError, meaning two different exception classes are used.
>
> ISTM StaticTuple_New should throw a ValueError, since you do
> StaticTuple_
> 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_
>
> ** Affects: bzr
> Importance: Undecided
> Status: New
>
> ** Description changed:
>
> Lookie:
>
> - if (size < 0) {
> - PyErr_BadIntern
> - return NULL;
> - }
> + if (size < 0) {
> + PyErr_BadIntern
> + return NULL;
> + }
>
> - if (size < 0 || size > 255) {
> - /* Too big or too small */
> - PyErr_SetString
> - " takes from 0 to 255 items");
> - return NULL;
> - }
> + if (size < 0 || size > 255) {
> + /* Too big or too small */
> + PyErr_SetString
> + " takes from 0 to 255 items");
> + return NULL;
> + }
>
> Not only is this redundant, but PyErr_BadIntern
> TypeError, meaning two different exception classes are used.
Actually, PyErr_BadIntern alCall raises a "SystemError": BadInternalCall (char *filename, int lineno) Format( PyExc_SystemErr or,
void
_PyErr_
{
PyErr_
...
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.
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.
> New(length) , and StaticTuple_ new_constructor should throw a new_constructor .
> ISTM StaticTuple_New should throw a ValueError, since you do
> StaticTuple_
> - 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_
>
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* unicode/ etc.)
of the argument passed is invalid. (Such as passing a dict rather than a
string/
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.)
John enigmail. mozdev. org/
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://
iEYEARECAAYFAkr gdB8ACgkQJdeBCY SNAANIfwCfV0Xlk fHbfFZ40MHOERph iscs tVTResb7LecruFE I7
51UAmwegOc60Jv9
=i+3B
-----END PGP SIGNATURE-----