Skip to content

gh-132742: Update documentation for the fcntl module #132765

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

Merged
merged 5 commits into from
Apr 23, 2025

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Apr 21, 2025

@AA-Turner
Copy link
Member

I’m not an expert on fcntl, did you just want a style/editorial review from me here?

A

@serhiy-storchaka
Copy link
Member Author

I would be grateful for a style/editorial review. I feel like there is some incorrect or unclear English here.

the *mutate_flag* parameter.

If it is false, the buffer's mutability is ignored and behaviour is as for a
read-only buffer, except that the 1024 byte limit mentioned above is avoided --
Copy link
Member Author

Choose a reason for hiding this comment

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

This was incorrect, the 1024 byte limit is not avoided if mutate_flag is false.

supplied buffer is less than 1024 bytes long it is first copied into a static
buffer 1024 bytes long which is then passed to :func:`ioctl` and copied back
into the supplied buffer.

If the :c:func:`ioctl` call fails, an :exc:`OSError` exception is raised.

.. note::
If the type or the size of *arg* does not match the type or the size
Copy link
Member Author

@serhiy-storchaka serhiy-storchaka Apr 21, 2025

Choose a reason for hiding this comment

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

In the C documentation, the type is int or a pointer to specific structure. The Python argument must be int or a bytes-like object of the corresponding size.

Copy link
Contributor

@StanFromIreland StanFromIreland left a comment

Choose a reason for hiding this comment

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

From a quick read

:func:`struct.pack`. The binary data is copied to a buffer whose address is
:class:`bytes` object, or a string.
The type and the size of *arg* must match the type and the size of
the argument of the operation as specified in the relevant C documentation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Link?

Copy link
Member Author

Choose a reason for hiding this comment

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

Depending on platform. Various manpages on Posix systems, some Microsoft links on Windows.

The parameter *arg* can be one of an integer, a :term:`bytes-like object`,
or a string.
The type and the size of *arg* must match the type and the size of
the argument of the operation as specified in the relevant C documentation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Link?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking the same thing, but since most fcntl values are platform-dependent there probably isn't a good documentation link to use here.

Comment on lines +158 to +159
this is most likely to result in a segmentation violation or
a more subtle data corruption.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
this is most likely to result in a segmentation violation or
a more subtle data corruption.
it is most likely to result in a segmentation violation or
a more subtle data corruption.

Continues on from previous suggestion

Copy link
Member Author

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

Thank you for your suggestions, @StanFromIreland.

I fixed my own writing, but some parts were just copied from the old documentation, and I am not sure that changing them will not introduce some subtle change in semantic.

:func:`struct.pack`. The binary data is copied to a buffer whose address is
:class:`bytes` object, or a string.
The type and the size of *arg* must match the type and the size of
the argument of the operation as specified in the relevant C documentation.
Copy link
Member Author

Choose a reason for hiding this comment

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

Depending on platform. Various manpages on Posix systems, some Microsoft links on Windows.

Comment on lines 111 to 116
If the type or the size of *arg* does not match the type or the size
of the argument of the operation (for example, if an integer is
passed when a pointer is expected, or the information returned in
the buffer by the operating system is larger than 1024 bytes),
this is most likely to result in a segmentation violation or
a more subtle data corruption.
Copy link
Member Author

Choose a reason for hiding this comment

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

This was mainly the moved old documentation.

Co-authored-by: Stan Ulbrych <[email protected]>
the argument is bytes it represents a binary structure, e.g. created by
:func:`struct.pack`. The binary data is copied to a buffer whose address is
header files. The argument *arg* can either be an integer value, a
:class:`bytes` object, or a string.
Copy link
Member

Choose a reason for hiding this comment

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

Looks like it accepts any readable buffer

Copy link
Member Author

Choose a reason for hiding this comment

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

It may accept other buffers without bf_releasebuffer, but bytes is the only standard type. Read-only memoryview or mmap are not supported.

@AA-Turner AA-Turner requested review from a team and removed request for AA-Turner April 21, 2025 21:31
Co-authored-by: Jelle Zijlstra <[email protected]>
Co-authored-by: Stan Ulbrych <[email protected]>
@serhiy-storchaka serhiy-storchaka merged commit 5f50541 into python:main Apr 23, 2025
24 checks passed
@github-project-automation github-project-automation bot moved this from Todo to Done in Docs PRs Apr 23, 2025
@miss-islington-app
Copy link

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@serhiy-storchaka serhiy-storchaka deleted the doc-fcntl branch April 23, 2025 11:27
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 23, 2025
@bedevere-app
Copy link

bedevere-app bot commented Apr 23, 2025

GH-132832 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Apr 23, 2025
serhiy-storchaka added a commit that referenced this pull request Apr 23, 2025
… (GH-132832)

(cherry picked from commit 5f50541)

Co-authored-by: Serhiy Storchaka <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants