Skip to content

bpo-20291: Fix MSVC warnings in getargs.c #27211

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 2 commits into from
Jul 17, 2021
Merged

bpo-20291: Fix MSVC warnings in getargs.c #27211

merged 2 commits into from
Jul 17, 2021

Conversation

Fidget-Spinner
Copy link
Member

@Fidget-Spinner Fidget-Spinner commented Jul 17, 2021 •
edited by bedevere-bot
Loading

@Fidget-Spinner
Copy link
Member Author

Fidget-Spinner commented Jul 17, 2021 •
edited
Loading

Currently:

Check warning on line 2555 in Python/getargs.c

GitHub Actions / Windows (x64)
Python/getargs.c#L2555
'=': conversion from 'Py_ssize_t' to 'int', possible loss of data [D:\a\cpython\cpython\PCbuild\pythoncore.vcxproj]

posonly is already int, so I think varargssize can be int without overflow.

isidentical
isidentical reviewed Jul 17, 2021
View reviewed changes
@isidentical
Copy link
Member

posonly is already int, so I think varargssize can be int without overflow.

Yep. As an extra guard we can just use Py_SAFE_DOWNCAST.

@Fidget-Spinner @isidentical
apply suggestions
6c1b377 
Co-Authored-By: Batuhan Taskaya <batuhan@python.org>
isidentical
isidentical approved these changes Jul 17, 2021
View reviewed changes
Copy link
Member

@isidentical isidentical left a comment

Choose a reason for hiding this comment

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

LGTM

@isidentical
Copy link
Member

test_asyncio failure, restarting the CI.

@isidentical isidentical merged commit f88e138 into python:main Jul 17, 2021
sthagen added a commit to sthagen/python-cpython that referenced this pull request Jul 17, 2021
@sthagen
Merge pull request #533 from python/main
0f61e2b 
bpo-20291: Fix MSVC warnings in getargs.c (pythonGH-27211)
@Fidget-Spinner Fidget-Spinner deleted the fix_msvc_warnings branch July 17, 2021 16:21
shihai1991 added a commit to shihai1991/cpython that referenced this pull request Jul 20, 2021
@shihai1991
Merge remote-tracking branch 'origin/main' into bpo_42035
56e8889 
* origin/main: (1146 commits)
  bpo-42064: Finalise establishing sqlite3 global state (pythonGH-27155)
  bpo-44678: Separate error message for discontinuous padding in binascii.a2b_base64 strict mode (pythonGH-27249)
  correct spelling (pythonGH-27076)
  bpo-44524: Add missed __name__ and __qualname__ to typing module objects (python#27237)
  bpo-27513: email.utils.getaddresses() now handles Header objects (python#13797)
  Clean up comma usage in Doc/library/functions.rst (python#27083)
  bpo-42238: Fix small rst issue in NEWS.d/. (python#27238)
  bpo-41972: Tweak fastsearch.h string search algorithms (pythonGH-27091)
  bpo-44340: Add support for building with clang full/thin lto (pythonGH-27231)
  bpo-44661: Update property_descr_set to use vectorcall if possible. (pythonGH-27206)
  bpo-44645: Check for interrupts on any potentially backwards edge (pythonGH-27216)
  bpo-41546: make pprint (like print) not write to stdout when it is None (pythonGH-26810)
  bpo-44554: refactor pdb targets (and internal tweaks) (pythonGH-26992)
  bpo-43086: Add handling for out-of-spec data in a2b_base64 (pythonGH-24402)
  bpo-44561: Update hyperlinks in Doc/distributing/index.rst (python#27032)
  bpo-42355: symtable.get_namespace() now checks whether there are multiple or any namespaces found (pythonGH-23278)
  bpo-44654: Do not export the union type related symbols (pythonGH-27223)
  bpo-44633: Fix parameter substitution of the union type with wrong types. (pythonGH-27218)
  bpo-44654: Refactor and clean up the union type implementation (pythonGH-27196)
  bpo-20291: Fix MSVC warnings in getargs.c (pythonGH-27211)
  ...
@vstinner
Copy link
Member

vstinner commented Sep 7, 2021

I dislike Py_SAFE_DOWNCAST(). Usually, I prefer to use a wider type (use Py_ssize_t for "i"), rather than using downcasting.

@vstinner
Copy link
Member

vstinner commented Sep 7, 2021

Well, in practice, a function should have less than 2^31 arguments :-)

@larryhastings larryhastings mentioned this pull request Apr 10, 2022
Open
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@isidentical isidentical isidentical approved these changes

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Fidget-Spinner @isidentical @vstinner @the-knights-who-say-ni @bedevere-bot