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

gh-93180: Update documentation of os.copy_file_range #93182

Merged
merged 10 commits into from
Jun 8, 2022
Merged

gh-93180: Update documentation of os.copy_file_range #93182

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c57e76f
Update docs about an old constraint of `os.copy_file_range`
illia-v May 24, 2022
23bbe3a
Add a note about a known issue of `os.copy_file_range`
illia-v May 24, 2022
1e38126
Document some of optimizations `os.copy_file_range` can use implicitly
illia-v May 24, 2022
caeb624
Add a news entry
illia-v May 24, 2022
59561bf
Update the note
illia-v May 30, 2022
bcb339d
Merge branch 'main' into fix-issue-93180
illia-v May 30, 2022
069651d
Explain what copying as if files are opened as binary means
illia-v Jun 6, 2022
f6152e4
Merge branch 'main' into fix-issue-93180
illia-v Jun 7, 2022
94ef9cc
Remove the news entry
illia-v Jun 7, 2022
7ab57e1
Apply Victor's suggestions
illia-v Jun 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add a note about a known issue of os.copy_file_range
  • Loading branch information
@illia-v
illia-v committed May 24, 2022
commit 23bbe3a5288ad133a7f50bc1e89b40945c5319d4
8 changes: 8 additions & 0 deletions Doc/library/os.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -812,6 +812,14 @@ as internal buffering of data.
The return value is the amount of bytes copied. This could be less than the
amount requested.

.. note::

This function should not be used for copying a file from special
filesystems like procfs and sysfs because of
`a known issue <https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/>`_.
Copy link
Member

Choose a reason for hiding this comment

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

The link contains a fix. Was the fix merged into Linux?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it wasn't. I tested os.copy_file_range for copying /proc/cpuinfo on Linux 5.17, it returned 0 and the destination file was empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW, some Python functions that operate files are prone to similar issues too :(. I'll create a few bug reports.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW, some Python functions that operate files are prone to similar issues too :(. I'll create a few bug reports.

#93300 and #93296 are two examples.

Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to not add a link to an email.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think about such a note?

:func:os.copy_file_range should not be used for copying a range of a pseudo file from a special filesystem like procfs and sysfs. It will always copy no bytes and return 0 as if the file was empty because of a known Linux kernel issue.

It will copy nothing and return 0 because file sizes there are not known
to the system.

.. availability:: Linux kernel >= 4.5 or glibc >= 2.27.

.. versionadded:: 3.8
Expand Down