raised exception is misleading

[kumma]

BPO	9041
Nosy	@vsajip, @amauryfa, @mdickinson, @abalkin, @ezio-melotti, @briancurtin, @meadori
Files	issue9041.patch: patch against main branch issue9041-v0.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2012-05-28.19:55:33.383>
created_at = <Date 2010-06-21.12:23:31.719>
labels = ['ctypes', 'type-bug']
title = 'raised exception is misleading'
updated_at = <Date 2012-05-28.19:55:33.382>
user = 'https://bugs.python.org/kumma'

bugs.python.org fields:

activity = <Date 2012-05-28.19:55:33.382>
actor = 'meador.inge'
assignee = 'none'
closed = True
closed_date = <Date 2012-05-28.19:55:33.383>
closer = 'meador.inge'
components = ['ctypes']
creation = <Date 2010-06-21.12:23:31.719>
creator = 'kumma'
dependencies = []
files = ['21690', '24518']
hgrepos = []
issue_num = 9041
keywords = ['patch', 'needs review']
message_count = 12.0
messages = ['108265', '108285', '108288', '108289', '108340', '108342', '133914', '143171', '143214', '152842', '153331', '161801']
nosy_count = 9.0
nosy_names = ['vinay.sajip', 'amaury.forgeotdarc', 'mark.dickinson', 'belopolsky', 'ezio.melotti', 'brian.curtin', 'meador.inge', 'kumma', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue9041'
versions = ['Python 2.7', 'Python 3.2', 'Python 3.3']

[kumma]

Python 2.6.4 (r264:75706, Dec  7 2009, 18:45:15) 
[GCC 4.4.1] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import ctypes
>>> ctypes.c_double(-10L)
c_double(-10.0)
>>> ctypes.c_double(-16158503035655503650357438344334975980222051334857742016065172713762327569433945446598600705761456731844358980460949009747059779575245460547544076193224141560315438683650498045875098875194826053398028819192033784138396109321309878080919047169238085235290822926018152521443787945770532904303776199561965192761047051351577287005728952652821735984108986866610276357675617870244379434256980638641483535093292326373531858799166625357628570460558031263448272521936380684506350754039141397287742159767011772534248723062567540713722795836229745796676103584020939467516975660579180998612463422664938089473950280866877864444041L)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError:  float expected instead of long instance

.. vs:

>>> float(-16158503035655503650357438344334975980222051334857742016065172713762327569433945446598600705761456731844358980460949009747059779575245460547544076193224141560315438683650498045875098875194826053398028819192033784138396109321309878080919047169238085235290822926018152521443787945770532904303776199561965192761047051351577287005728952652821735984108986866610276357675617870244379434256980638641483535093292326373531858799166625357628570460558031263448272521936380684506350754039141397287742159767011772534248723062567540713722795836229745796676103584020939467516975660579180998612463422664938089473950280866877864444041L)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OverflowError: long int too large to convert to float

[merwok]

Thanks for your report. Unfortunately, 2.6 and 3.1 are stable releases, they only get security and documentation fixes. 2.7 is nearly in the same state, since it’s at the release candidate stage. Can you check if your bug still applies to 3.2 (branch named “py3k”) and propose a patch? Thanks again.

[briancurtin]

"they only get security and documentation fixes"

2.6 does receive bug fixes. 2.5 is the version in security fix only mode.

[merwok]

Thanks Brian, I’ll note that somewhere and be exact in the future.
*has to check the bugs he’s nosy on now to correct comments*

[kumma]

I'm a newbie what it comes to Python's C-sources, so please do not
take me too seriously.
I fetched the sources 'svn checkout
http://svn.python.org/projects/python/branches/release26-maint '
studied the issue, and my best guess is that
Modules/_ctypes/cfield.c 's funktion d_set is called, when converting
something to ctypes.c_double

Please check, that this is what happens :)

If so, Objects/longobject.c's function double PyLong_AsDouble(PyObject
*vv) sets the overflow exception, but d_set overwrites it, like you
can see:

static PyObject *
d_set(void *ptr, PyObject *value, Py_ssize_t size)
{
    double x;

    x = PyFloat_AsDouble(value);
    if (x == -1 && PyErr_Occurred()) {
        PyErr_Format(PyExc_TypeError,
                     " float expected instead of %s instance",
                     value->ob_type->tp_name);
        return NULL;
    }
    memcpy(ptr, &x, sizeof(double));
    _RET(value);
}

Perhaps something like:
if (PyErr_ExceptionMatches(PyExc_OverflowError)){
	return NULL;
}

just after the line:
'f (x == -1 && PyErr_Occurred()) {'
could fix this?

But like I said, I'm an newbie, this was actually my first look into
Python/C API and I have not tested this fix in any way whatsoever

[kumma]

If d_set makes the conversion, py3k does the same thing:

http://svn.python.org/projects/python/branches/py3k/Modules/_ctypes/cfield.c

It has a function d_set_sw, which might have same issues as well. The
same function can be found from 2.6 too:
http://svn.python.org/projects/python/branches/release26-maint/Modules/_ctypes/cfield.c

[meadori]

This problem still occurs in the main development branch. I think the
'PyErr_ExceptionMatches' check that Pauli suggested will work. The same
problem exist for 'c_float', 'c_double', and 'c_longdouble'.

Attached is a patch that fixes the issue and includes covering tests.
Tested on OS X 10.6.5.

[vsajip]

While the patch might improve over the current situation, doesn't it potentially mask other errors which might be raised by PyFloat_AsDouble()?
Why not just

x = PyFloat_AsDouble(value);
if (PyErr_Occurred())
    return NULL;

which would not mask any exception?

[meadori]

That is a good question. While it is true that errors other than 'PyExc_OverflowError', will be mapped onto a 'TypeError' I don't think that is a bad thing. Any errors that come out of 'PyFloat_AsDouble' should be handled on a case-by-case basis and not blindly passed back out the call chain. Otherwise, we may end up passing back errors (which are who knows what) that make sense for a caller of 'PyFloat_AsDouble', but not for callers of 'g_set'.

Also, the interface would become variable, meaning that whenever 'PyFloat_AsDouble' introduces new exceptions, then this code would too, which would lead to a somewhat unpredictable interface for callers of 'g_set'.

[kumma]

Could we overcome these issues by some kind of exception inheritance?

On Tue, Aug 30, 2011 at 5:28 PM, Meador Inge <report@bugs.python.org> wrote:

Meador Inge <meadori@gmail.com> added the comment:

That is a good question.  While it is true that errors other than 'PyExc_OverflowError', will be mapped onto a 'TypeError' I don't think that is a bad thing.  Any errors that come out of 'PyFloat_AsDouble' should be handled on a case-by-case basis and not blindly passed back out the call chain.  Otherwise, we may end up passing back errors (which are who knows what) that make sense for a caller of 'PyFloat_AsDouble', but not for callers of 'g_set'.

Also, the interface would become variable, meaning that whenever 'PyFloat_AsDouble' introduces new exceptions, then this code would too, which would lead to a somewhat unpredictable interface for callers of 'g_set'.

----------

---

Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue9041\>

---

[meadori]

After thinking about it a bit more I am OK with Vinay's proposal. Attached is an updated patch.

Also, I also noticed that the 'struct' module has the same problem:

>>> big_int = int(sys.float_info.max) * 2
>>> struct.pack('d', big_int)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
struct.error: required argument is not a float

but the 'array' module does the right thing:

>>> big_int = int(sys.float_info.max) * 2
[68068 refs]
>>> array.array('d', [big_int])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OverflowError: long int too large to convert to float
[68068 refs]
>>> array.array('d', [""])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: a float is required

Mark, do you have any opinions on the error handling here and in the struct module?