Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-129594: Remove redundant check on varargs in _PyArg_CheckPositional#129595

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
picnixz merged 3 commits intopython:mainfromeendebakpt:_PyArg_CheckPositional
May 26, 2025

Conversation

eendebakpt
Copy link
Contributor

@eendebakpteendebakpt commentedFeb 2, 2025•
edited by bedevere-appbot
Loading

@eendebakpteendebakpt requested a review frompicnixzApril 4, 2025 13:42
Copy link
Member

@picnixzpicnixz left a comment•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This check is indeed unnecessary but can you transform it into an assertion? just so that we're sure we don't call the function incorrectly or is it impossible to add some assertion inside this?

@eendebakpt
Copy link
ContributorAuthor

I think we could use an assertion within the macro. But if there is no reason to have it, it can only cause confusion.

The code in question was moved around a bit by Victor Stinner (only refactoring, functionality was not touched afaics).
The check was introduced inhttps://github.com/python/cpython/pull/18609/files (under a slightly different nameANY_VARARGS). From that PR it is not clear to me why it was added.@isidentical As PR author do you know why the check was added?

@eendebakpteendebakpt requested a review frompicnixzMay 23, 2025 21:22
@picnixzpicnixz self-assigned thisMay 23, 2025
@picnixz
Copy link
Member

I'll merge this one tomorrow, sorry for forgetting about this one :')

eendebakpt reacted with thumbs up emoji

@picnixz
Copy link
Member

picnixz commentedMay 24, 2025•
edited
Loading

ThePY_SSIZE_T_MAX check might be because of argument clinic:

staticPyObject*sys_audit(PyObject*module,PyObject*const*args,Py_ssize_tnargs){PyObject*return_value=NULL;constchar*event;PyObject*__clinic_args=NULL;if (!_PyArg_CheckPositional("audit",nargs,1,PY_SSIZE_T_MAX)) {        gotoexit;    }

In the situation above, we want to check that nargs is at least 1. However, because of the!= PY_SSIZE_T_MAX, we're actually making a real call to_PyArg_CheckPositional even in the success case, which is not ideal.

So I don't think we need the check at all as it doesn't help us (and even slows us down whensys.audit("abc") is correct)

@picnixz
Copy link
Member

arf, I forgot to merge this one.@eendebakpt can you confirm my analysis? I may have done it while I was just waking up and now I'm about to sleep :') (sorry, tomorrow itwill be merged!)

@eendebakpt
Copy link
ContributorAuthor

Don't ask for analysisand promise to merge tomorrow, you might end up in an awkward situation ;-)

I agree with your analysis. ThePY_SSIZE_T_MAX is a signal from argument clinic when have a variable number of arguments (e.g.*args). This is defined here:

NO_VARARG:Final[str]="PY_SSIZE_T_MAX"

max_args=NO_VARARGifself.varposelseself.max_pos

@picnixz
Copy link
Member

Ok thanks.

To summarize_PyArg_CheckPositional(funcname, n, 1, PY_SSIZE_T_MAX) == 1 forn < PY_SSIZE_T_MAX and_PyArg_CheckPositional(funcname, PY_SSIZE_T_MAX, 1, PY_SSIZE_T_MAX) == 1 required an additional function call before this change, while now they can just use the(min) <= (nargs) && (nargs) <= (max) check.

@picnixzpicnixz merged commitfb09db1 intopython:mainMay 26, 2025
45 checks passed
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure⚠️⚠️⚠️

Hi! The buildbotaarch64 Android 3.x (tier-3) has failed when building commitfb09db1.

What do you need to do:

  1. Don't panic.
  2. Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1594/builds/2507) and take a look at the build logs.
  4. Check if the failure is related to this commit (fb09db1) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1594/builds/2507

Summary of the results of the build (if available):

Click to see traceback logs
remote:Enumerating objects: 9, done.remote:Counting objects:  11% (1/9)remote:Counting objects:  22% (2/9)remote:Counting objects:  33% (3/9)remote:Counting objects:  44% (4/9)remote:Counting objects:  55% (5/9)remote:Counting objects:  66% (6/9)remote:Counting objects:  77% (7/9)remote:Counting objects:  88% (8/9)remote:Counting objects: 100% (9/9)remote:Counting objects: 100% (9/9), done.remote:Compressing objects:  12% (1/8)remote:Compressing objects:  25% (2/8)remote:Compressing objects:  37% (3/8)remote:Compressing objects:  50% (4/8)remote:Compressing objects:  62% (5/8)remote:Compressing objects:  75% (6/8)remote:Compressing objects:  87% (7/8)remote:Compressing objects: 100% (8/8)remote:Compressing objects: 100% (8/8), done.remote:Total 9 (delta 2), reused 1 (delta 1), pack-reused 0 (from 0)From https://github.com/python/cpython * branch                    main       -> FETCH_HEADNote:switching to 'fb09db1b934a51e52e46dc64d7e5e84fe69e6160'.You are in 'detached HEAD' state. You can look around, make experimentalchanges and commit them, and you can discard any commits you make in thisstate without impacting any branches by switching back to a branch.If you want to create a new branch to retain commits you create, you maydo so (now or later) by using -c with the switch command. Example:  git switch -c <new-branch-name>Or undo this operation with:  git switch -Turn off this advice by setting config variable advice.detachedHead to falseHEAD is now at fb09db1b934 gh-129594: Remove redundant check on varargs in `_PyArg_CheckPositional` (#129595)Switched to and reset branch 'main'configure:WARNING: no system libmpdecimal found; falling back to bundled libmpdecimal (deprecated and scheduled for removal in Python 3.15)configure:WARNING: pkg-config is missing. Some dependencies may not be detected correctly.  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current                                 Dload  Upload   Total   Spent    Left  Speed  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0100  192k  100  192k    0     0   255k      0 --:--:-- --:--:-- --:--:--  255k  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current                                 Dload  Upload   Total   Spent    Left  Speed  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0100 42455  100 42455    0     0  93544      0 --:--:-- --:--:-- --:--:-- 93544  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current                                 Dload  Upload   Total   Spent    Left  Speed  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0100 5041k  100 5041k    0     0  7181k      0 --:--:-- --:--:-- --:--:-- 7181k  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current                                 Dload  Upload   Total   Spent    Left  Speed  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0100 1252k  100 1252k    0     0  1520k      0 --:--:-- --:--:-- --:--:-- 1520k  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current                                 Dload  Upload   Total   Spent    Left  Speed  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0100  635k  100  635k    0     0  1841k      0 --:--:-- --:--:-- --:--:-- 1841k../../configure:line 4071: pkg-config: command not foundconfigure:WARNING: no system libmpdecimal found; falling back to bundled libmpdecimal (deprecated and scheduled for removal in Python 3.15)configure:WARNING: pkg-config is missing. Some dependencies may not be detected correctly.../../Python/fileutils.c:460:1: warning: unused function 'decode_current_locale' [-Wunused-function]  460 | decode_current_locale(const char* arg, wchar_t **wstr, size_t *wlen,|^~~~~~~~~~~~~~~~~~~~~../../Python/fileutils.c:679:1: warning: unused function 'encode_current_locale' [-Wunused-function]  679 | encode_current_locale(const wchar_t *text, char **str,|^~~~~~~~~~~~~~~~~~~~~2 warnings generated.../../Modules/_localemodule.c:148:1: warning: unused function 'is_all_ascii' [-Wunused-function]  148 | is_all_ascii(const char *str)|^~~~~~~~~~~~1 warning generated.../../Modules/_hacl/Lib_Memzero0.c:66:6: warning: "Your platform does not support any safe implementation of memzero -- consider a pull request!" [-W#warnings]   66 |     #warning "Your platform does not support any safe implementation of memzero -- consider a pull request!"|^1 warning generated.  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current                                 Dload  Upload   Total   Spent    Left  Speed  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0100  8784  100  8784    0     0  89466      0 --:--:-- --:--:-- --:--:-- 89632  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current                                 Dload  Upload   Total   Spent    Left  Speed  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:02 --:--:--     0  0    22    0     0    0     0      0      0 --:--:--  0:00:02 --:--:--     0curl:(56) The requested URL returned error: 429Warning:Problem (retrying all errors). Will retry in 1 seconds. 5 retriesWarning:left.  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:02 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:03 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:04 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:05 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:06 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:07 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:08 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:09 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:10 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:12 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:13 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:14 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:15 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:16 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:17 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:18 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:19 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:20 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:22 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:23 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:24 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:25 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:26 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:27 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:28 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:29 --:--:--     0  0   451    0     0    0     0      0      0 --:--:--  0:00:30 --:--:--     0curl:(56) The requested URL returned error: 503Warning:Problem (retrying all errors). Will retry in 2 seconds. 4 retriesWarning:left.  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0100  2894  100  2894    0     0  39305      0 --:--:-- --:--:-- --:--:-- 39643  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current                                 Dload  Upload   Total   Spent    Left  Speed  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:02 --:--:--     0  0    22    0     0    0     0      0      0 --:--:--  0:00:02 --:--:--     0curl:(56) The requested URL returned error: 429Warning:Problem (retrying all errors). Will retry in 1 seconds. 5 retriesWarning:left.  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:02 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:03 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:04 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:05 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:06 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:07 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:08 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:09 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:11 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:12 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:13 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:14 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:15 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:16 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:17 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:18 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:20 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:21 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:22 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:23 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:24 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:25 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:26 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:27 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:29 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:30 --:--:--     0  0   452    0     0    0     0      0      0 --:--:--  0:00:30 --:--:--     0curl:(56) The requested URL returned error: 503Warning:Problem (retrying all errors). Will retry in 2 seconds. 4 retriesWarning:left.  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:02 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:03 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:04 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:05 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:06 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:07 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:08 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:09 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:10 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:11 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:13 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:14 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:15 --:--:--     0  0   452    0     0    0     0      0      0 --:--:--  0:00:15 --:--:--     0curl:(56) The requested URL returned error: 503Warning:Problem (retrying all errors). Will retry in 4 seconds. 3 retriesWarning:left.  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:02 --:--:--     0  0    22    0     0    0     0      0      0 --:--:--  0:00:02 --:--:--     0curl:(56) The requested URL returned error: 429Warning:Problem (retrying all errors). Will retry in 8 seconds. 2 retriesWarning:left.  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:02 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:03 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:04 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:05 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:06 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:07 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:08 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:10 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:11 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:12 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:13 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:14 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:15 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:16 --:--:--     0  0    22    0     0    0     0      0      0 --:--:--  0:00:17 --:--:--     0curl:(56) The requested URL returned error: 429Warning:Problem (retrying all errors). Will retry in 16 seconds. 1 retriesWarning:left.  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:02 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:03 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:04 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:05 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:06 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:07 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:08 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:09 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:10 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:12 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:13 --:--:--     0  0     0    0     0    0     0      0      0 --:--:--  0:00:14 --:--:--     0  0   452    0     0    0     0      0      0 --:--:--  0:00:15 --:--:--     0curl:(56) The requested URL returned error: 503

@mhsmith
Copy link
Member

This was caused by a general GitHub outage:https://www.githubstatus.com/incidents/d0nm3xcdc5jw

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@skirpichevskirpichevskirpichev approved these changes

@picnixzpicnixzpicnixz approved these changes

@ericsnowcurrentlyericsnowcurrentlyAwaiting requested review from ericsnowcurrentlyericsnowcurrently is a code owner

Assignees

@picnixzpicnixz

Labels
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

5 participants
@eendebakpt@picnixz@bedevere-bot@mhsmith@skirpichev
[8]ページ先頭
©2009-2025 Movatter.jp