Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Duplicated st_ino calculation #101810

Closed
juria90 opened this issue Feb 10, 2023 · 2 comments
Closed

Duplicated st_ino calculation #101810

juria90 opened this issue Feb 10, 2023 · 2 comments
Labels
3.10 only security fixes 3.11 only security fixes 3.12 bugs and security fixes OS-windows type-feature A feature request or enhancement

Comments

@juria90
Copy link
Contributor

juria90 commented Feb 10, 2023 •
edited by bedevere-bot
Loading

Feature or enhancement

The status->st_ino = (((uint64_t)info.nFileIndexHigh) << 32) + info.nFileIndexLow; in function _Py_fstat_noraise() in file Python/fileutils.c is already calculated and assigned in the function _Py_attribute_data_to_stat() in same file.
So I'm proposing removing the duplicated code.

Pitch

It's a duplicated code that is already done in the previous function call _Py_attribute_data_to_stat().

Previous discussion

Linked PRs
@juria90 juria90 added the type-feature A feature request or enhancement label Feb 10, 2023
@eryksun eryksun added 3.11 only security fixes 3.10 only security fixes 3.12 bugs and security fixes OS-windows labels Feb 10, 2023
@eryksun
Copy link
Contributor

eryksun commented Feb 10, 2023

This redundant calculation has been in the source for a very long time. 😳

In 3.1, attribute_data_to_stat() in "Modules/posixmodule.c" was based on a WIN32_FILE_ATTRIBUTE_DATA record, and win32_fstat() didn't use attribute_data_to_stat().

In 3.2, attribute_data_to_stat() was updated to use a BY_HANDLE_FILE_INFORMATION record, and win32_fstat() was updated to call attribute_data_to_stat(). A redundant st_ino calculation was left in place, however.

In 3.5, win32_fstat() was moved to _Py_fstat_noraise() in "Python/fileutils.c", but apparently no one noticed the redundant calculation.

zooba pushed a commit that referenced this issue Feb 13, 2023
@juria90
gh-101810: Remove duplicated st_ino calculation (GH-101811)
95cbb3d
carljm added a commit to carljm/cpython that referenced this issue Feb 13, 2023
@carljm
Merge branch 'main' into inlinecomp2
ae0bd02 
* main:
  pythongh-101810: Remove duplicated st_ino calculation (pythonGH-101811)
  pythongh-92547: Purge sqlite3_enable_shared_cache() detection from configure (python#101873)
  pythonGH-100987: Refactor `_PyInterpreterFrame` a bit, to assist generator improvement. (pythonGH-100988)
  pythonGH-87849: Simplify stack effect of SEND and specialize it for generators and coroutines. (pythonGH-101788)
  Correct trivial grammar in reset_mock docs (python#101861)
  pythongh-101845: pyspecific: Fix i18n for availability directive (pythonGH-101846)
  pythongh-89792: Limit test_tools freeze test build parallelism based on the number of cores (python#101841)
  pythongh-85984: Utilize new "winsize" functions from termios in pty tests. (python#101831)
  pythongh-89792: Prevent test_tools from copying 1000M of "source" in freeze test (python#101837)
  Fix typo in test_fstring.py (python#101823)
  pythonGH-101797: allocate `PyExpat_CAPI` capsule on heap (python#101798)
  pythongh-101390: Fix docs for `imporlib.util.LazyLoader.factory` to properly call it a class method (pythonGH-101391)
@OTheDev
Copy link
Contributor

OTheDev commented Feb 14, 2023

@juria90 Can this issue be closed?

@zooba zooba closed this as completed Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
3.10 only security fixes 3.11 only security fixes 3.12 bugs and security fixes OS-windows type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@eryksun @zooba @juria90 @OTheDev