New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
wrong error messages when using PyArg_ParseTuple to parse normal tuples #72448
Comments
------------ current state ------------ For example, such case in Modules/socketmodule.c in socket_getnameinfo leads to the following:
>>> import socket
>>> socket.getnameinfo(tuple(), 1)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: function takes at least 2 arguments (0 given)
>>> ------------ proposed changes ------------ ------------ diff ------------ ------------ tests ------------ In addition, I ran 'python_d.exe -m test -j3' (on my 64-bit Windows 10) with and without the patches, and got quite the same output. |
I dislike passing a error message in the function name. I suggest to instead raise a new exception with a better error message and chain it with the previous exception. |
Do you mean that in each case PyArg_ParseTuple fails, we should chain |
An error message is not passed in the function name. PyArg_ParseTuple() allows you to pass an arbitrary error message. I think this feature is specially designed for these cases. I didn't look the patch close, but Oren's approach LGTM in general. |
Oren, could you write reproducers for all affected cases? I don't think we need to add them as regular tests, but we should be able to check changes manually. There are conflicts in Modules/itertoolsmodule.c. |
-0 The new code looks awful, and in the case of the itertools module, these are messages that users are unlikely to see (the methods are typically only called by pickle, and pickling itself is rare for itertools). |
Would it look less awful if remove "xxxxxx.__setstate__: "? |
Raymond? any suggestions for how to make the code less ugly? |
I suggest to withdraw changes in itertoolsmodule.c and avoid using "dunder" names like __setstate__ and __new__ in error messages. |
as Serhiy pointed out in PR 668, here are some more functions that produce the ISTM that searching for 'PyTuple_Check(' might be a good way to find more such |
I replied to your comments in Rietveld, Serhiy. (http://bugs.python.org/review/28261) also, i found two places with a quite similar issue:
- in Objects/exceptions.c in ImportError_init:
>>> ImportError(1, 2, 3, 4, a=5, b=6, c=7)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: ImportError() takes at most 2 arguments (3 given)
- in Python/bltinmodule.c in min_max:
>>> min(1, 2, 3, 4, a=5, b=6, c=7)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: function takes at most 2 arguments (3 given) may I fix them also as part of this issue? |
After more looking, I found this issue in two more places:
- in Modules/itertoolsmodule.c in product_new:
>>> itertools.product(0, a=1, b=2, c=3, d=4, e=5, f=6)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: product() takes at most 1 argument (6 given)
- in Python/bltinmodule.c in builtin_print:
>>> print(0, a=1, b=2, c=3, d=4, e=5, f=6)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: print() takes at most 4 arguments (6 given) what do you think? |
This looks as a different issue to me. All these cases are similar, but different from the original issue. Please open a new issue. |
Answered questions on Rietveld. I'm not sure email notification from Rietveld works now. |
it seems that I have missed some places which are part of this issue, at least also, when fixing these, we should also add a check before the call to |
Oren, do you mind to backport the part of your changes that adds explicit checks for tuples to 3.6 and 2.7? Raising SystemError looks like a bug to me. |
sure |
Thank you for your contribution Oren! |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: