| msg208189 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2014-01-15 20:24 |
| The code in Modules/_sqlite/connection.c is sloppy. The functions pysqlite_connection_execute, pysqlite_connection_executemany, and pysqlite_connection_executescript accept a third "PyObject *kwargs". However none of these functions are marked METH_KEYWORD. This only works because the kwargs parameter is actually ignored--the functions only support positional-only arguments. Obviously the "PyObject *kwargs" parameters should be removed for these three functions. A slightly more advanced problem: pysqlite_connection_call, which implements sqlite3.Connection.__call__(), ignores its kwargs parameter completely. If it doesn't accept keyword parameters it should at least complain if any are passed in. Georg: you want this fixed in 3.3? 3.2? Benjamin: you want this fixed in 2.7? |
|
|
| msg208204 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2014-01-15 22:21 |
| Why do you want to fix it in order versions? Can it lead to a crash? For __call__, it seems to me we should do a deprecation and remove it in 3.5. Otherwise we'll risk breaking working code for no good reason (working code with useless parameters, but still working code :) |
|
|
| msg208224 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-01-16 00:06 |
| You're right, deprecation sounds best. If Georg or Benjamin want the fix in earlier releases I'll split the pysqlite_connection_call into another issue, otherwise I won't bother. As for fixing the undefined behavior in older versions: since its behavior is undefined, yes, it *could* cause a crash. But by that token it *could* teleport you to Mars and give you a funny-looking nose. In practice it *should* be utterly harmless, as I believe every platform supported by Python 3.4 uses the caller-pops-stack calling convention. And we've lived with it for this long, so it doesn't seem to be hurting anything. But like all undefined behavior I can make no guarantee. MvL in particular comes down like a ton of bricks whenever someone proposes checking in code that's technically undefined behavior. I've had the relevant chapter and verse of the C standard quoted at me for this exact thing (calling a function pointer using a sliiiightly different signature than the actual function). |
|
|
| msg242766 - (view) |
Author: Larry Hastings (larry) * |
Date: 2015-05-08 14:25 |
| I'm gonna fix this now. (I'm cleaning up some old issues I filed on the bug tracker this morning.) For 3.4, I'm just removing the PyObject *kwargs for those three functions that don't actually accept keyword arguments (METH_VARARGS) and aren't passed that parameter. That's a bug plain and simple, it's relying on undefined behavior, and it's better that we fix it. For 3.5. I'm adding a call to _PyArg_NoKeywords() to pysqlite_connection_call. Previously it simply ignored any/all keyword arguments; now it will complain if it is passed any. We don't need a deprecation cycle for that. |
|
|
| msg242768 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2015-05-08 14:45 |
| New changeset 4c860369b6c2 by Larry Hastings in branch '3.4': Issue #20274: Remove ignored and erroneous "kwargs" parameters from three https://hg.python.org/cpython/rev/4c860369b6c2 New changeset 3e9f4f3c7fa7 by Larry Hastings in branch 'default': Issue #20274: When calling a _sqlite.Connection, it now complains if passed https://hg.python.org/cpython/rev/3e9f4f3c7fa7 |
|
|
| msg242769 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2015-05-08 14:47 |
| Is 2.7 affected? |
|
|
| msg242770 - (view) |
Author: Larry Hastings (larry) * |
Date: 2015-05-08 14:49 |
| Yes, all those bugs exist in 2.7. However, Benjamin hasn't responded to this bug, so I assume he doesn't care and I should leave 2.7 alone. |
|
|
| msg242771 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2015-05-08 15:01 |
| I think the bug should be fixed in 2.7 (but not in 3.3 unless we get a crasher). |
|
|
| msg242772 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2015-05-08 15:11 |
| I don't mind if you fix it in 2.7, too. (Sorry, I get a lot of bug related emails...) |
|
|
| msg242774 - (view) |
Author: Larry Hastings (larry) * |
Date: 2015-05-08 16:08 |
| Benjamin: I assume you want the extraneous (and undefined behavior) kwargs parameters removed. Do you also want pysqlite_connection_call() to start calling _PyArg_NoKeywords()? |
|
|
| msg242775 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2015-05-08 16:40 |
| On Fri, May 8, 2015, at 12:08, Larry Hastings wrote: > > Larry Hastings added the comment: > > Benjamin: I assume you want the extraneous (and undefined behavior) > kwargs parameters removed. > > Do you also want pysqlite_connection_call() to start calling > _PyArg_NoKeywords()? Yes, please. |
|
|
| msg242776 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2015-05-08 16:56 |
| New changeset c91d135b0776 by Larry Hastings in branch '2.7': Issue #20274: When calling a _sqlite.Connection, it now complains if passed https://hg.python.org/cpython/rev/c91d135b0776 |
|
|