classification
Title: raised exception is misleading
Type: behavior Stage: resolved
Components: ctypes Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: amaury.forgeotdarc, belopolsky, brian.curtin, ezio.melotti, kumma, mark.dickinson, meador.inge, python-dev, vinay.sajip
Priority: normal Keywords: needs review, patch

Created on 2010-06-21 12:23 by kumma, last changed 2012-05-28 19:55 by meador.inge. This issue is now closed.

Files
File name Uploaded Description Edit
issue9041.patch meador.inge, 2011-04-17 00:13 patch against main branch review
issue9041-v0.patch meador.inge, 2012-02-14 05:44 review
Messages (12)
msg108265 - (view) Author: Pauli Rikula (kumma) Date: 2010-06-21 12:23
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
msg108285 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-06-21 16:30
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.
msg108288 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-06-21 17:04
"they only get security and documentation fixes"

2.6 does receive bug fixes. 2.5 is the version in security fix only mode.
msg108289 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-06-21 17:07
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*
msg108340 - (view) Author: Pauli Rikula (kumma) Date: 2010-06-22 07:43
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
msg108342 - (view) Author: Pauli Rikula (kumma) Date: 2010-06-22 08:15
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
msg133914 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2011-04-17 00:13
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.
msg143171 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2011-08-29 18:27
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?
msg143214 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2011-08-30 14:27
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'.
msg152842 - (view) Author: Pauli Rikula (kumma) Date: 2012-02-08 09:27
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>
> _______________________________________
msg153331 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2012-02-14 05:44
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?
msg161801 - (view) Author: Roundup Robot (python-dev) Date: 2012-05-28 19:52
New changeset 8a65eceea56c by Meador Inge in branch '2.7':
Issue #9041: raised exception is misleading
http://hg.python.org/cpython/rev/8a65eceea56c

New changeset a2e140b083e0 by Meador Inge in branch '3.2':
Issue #9041: raised exception is misleading
http://hg.python.org/cpython/rev/a2e140b083e0

New changeset 7c7bbf4a70b7 by Meador Inge in branch 'default':
Issue #9041: raised exception is misleading
http://hg.python.org/cpython/rev/7c7bbf4a70b7
History
Date User Action Args
2012-05-28 19:55:33meador.ingesetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2012-05-28 19:52:25python-devsetnosy: + python-dev
messages: + msg161801
2012-02-29 12:35:58ezio.melottisetkeywords: + needs review
2012-02-14 05:44:51meador.ingesetfiles: + issue9041-v0.patch
versions: + Python 2.7, Python 3.2
nosy: + mark.dickinson

messages: + msg153331
2012-02-08 09:27:53kummasetmessages: + msg152842
2011-08-30 15:44:11eric.araujosetnosy: - eric.araujo
2011-08-30 14:28:00meador.ingesetmessages: + msg143214
2011-08-29 18:27:00vinay.sajipsetnosy: + vinay.sajip
messages: + msg143171
2011-04-17 11:09:14ezio.melottisetnosy: + ezio.melotti
2011-04-17 00:13:55meador.ingesetfiles: + issue9041.patch

assignee: theller ->
versions: + Python 3.3, - Python 2.6
keywords: + patch
nosy: + belopolsky, amaury.forgeotdarc, meador.inge, - theller

messages: + msg133914
stage: patch review
2010-06-22 08:15:07kummasetmessages: + msg108342
2010-06-22 07:43:57kummasetmessages: + msg108340
2010-06-21 17:07:12eric.araujosetmessages: + msg108289
2010-06-21 17:04:29brian.curtinsetnosy: + brian.curtin
messages: + msg108288
2010-06-21 16:30:21eric.araujosetnosy: + eric.araujo
messages: + msg108285
2010-06-21 12:23:31kummacreate