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.

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

C API: Add PyLong_FileDescriptor_Converter(): make private _PyLong_FileDescriptor_Converter() public #116646

Closed
vstinner opened this issue Mar 12, 2024 · 3 comments
Labels
topic-C-API type-feature A feature request or enhancement

Comments

@vstinner
Copy link
Member

vstinner commented Mar 12, 2024

I propose to make the private _PyLong_FileDescriptor_Converter() function public and add it to the limited C API (version 3.13).

The function is used used by 4 stdlib C extensions: posix, fcntl, select and termios. Making the function public helps to build these extensions with the limited C API.

In the PyPI top 5,000 projects (at 2023-11-15), no project uses _PyLong_FileDescriptor_Converter(), but 25 projects use directly PyObject_AsFileDescriptor():

  • Cython (3.0.5)
  • M2Crypto (0.40.1)
  • astropy (5.3.4)
  • cassandra-driver (3.28.0)
  • catboost (1.2.2)
  • cffi (1.16.0)
  • igraph (0.11.2)
  • line_profiler (4.1.2)
  • mod_wsgi (4.9.4)
  • numpy (1.26.2)
  • orjson (3.9.10)
  • pycups (2.0.1)
  • pydoop (2.0.0)
  • pygame (2.5.2)
  • pygraphviz (1.11)
  • pysam (0.22.0)
  • python-sat-0.1.8.dev10
  • python-vlc (3.0.20123)
  • pyuwsgi (2.0.23)
  • pyxattr (0.8.1)
  • scylla-driver (3.26.3)
  • ssh-python (1.0.0)
  • ssh2-python (1.0.0)
  • uwsgi (2.0.23)
  • yara-python (4.3.1)

To convert an object to a file descriptor, Argument Clinic generates code calling _PyLong_FileDescriptor_Converter().

This function is quite simple, it's a thin wrapper to the public PyObject_AsFileDescriptor() function:

int
_PyLong_FileDescriptor_Converter(PyObject *o, void *ptr)
{
    int fd = PyObject_AsFileDescriptor(o);
    if (fd == -1) {
        return 0;
    }
    *(int *)ptr = fd;
    return 1;
}

PyObject_AsFileDescriptor(obj) converts Python integer to a C int, or call obj.fileno() and converts the result to a C int, or raise a TypeError.

The API for converter callback is already exposed in PyArg_ParseTuple(): see O& format documentation: https://docs.python.org/dev/c-api/arg.html#other-objects

There is already PyUnicode_FSConverter() in the limited C API.

Linked PRs

@encukou
Copy link
Member

encukou commented Mar 13, 2024

I don't think we need public API for a wrapper that adapts an existing public function to what AC needs. I don't see how this helps users.

Could AC be changed to emit those few lines directly?

vstinner added a commit to vstinner/cpython that referenced this issue Mar 13, 2024
Argument Clinic now calls directly PyObject_AsFileDescriptor() for
the "fildes" format.

Add tests on the "fildes" format in _testclinic_limited.
vstinner added a commit to vstinner/cpython that referenced this issue Mar 13, 2024
Argument Clinic now calls directly PyObject_AsFileDescriptor() for
the "fildes" format.

Add tests on the "fildes" format in _testclinic_limited.
@vstinner
Copy link
Member Author

@encukou:

Could AC be changed to emit those few lines directly?

That sounds like a great idea: I wrote PR #116736 for that. It makes the generated code compatible with the limited C API.

I didn't realize that Argument Clinic was already being used in CPython internals to generate code using the private _PyLong_FileDescriptor_Converter() helper function.

vstinner added a commit to vstinner/cpython that referenced this issue Mar 13, 2024
Add tests on the "fildes" converter in _testclinic_limited.
vstinner added a commit to vstinner/cpython that referenced this issue Mar 13, 2024
Add tests on the "fildes" converter to _testclinic_limited.
vstinner added a commit that referenced this issue Mar 14, 2024
Add tests on the "fildes" converter to _testclinic_limited.
@vstinner
Copy link
Member Author

Fixed by d402872

vstinner added a commit to vstinner/cpython that referenced this issue Mar 14, 2024
Only add includes when the converter is effectively used.
vstinner added a commit that referenced this issue Mar 14, 2024
Only add includes when the converter is effectively used.
vstinner added a commit to vstinner/cpython that referenced this issue Mar 14, 2024
The fildes converter of Argument Clinic now always call
PyObject_AsFileDescriptor(), not only for the limited C API.

The _PyLong_FileDescriptor_Converter() converter stays as a fallback
when PyObject_AsFileDescriptor() cannot be used.
vstinner added a commit that referenced this issue Mar 14, 2024
)

The fildes converter of Argument Clinic now always call
PyObject_AsFileDescriptor(), not only for the limited C API.

The _PyLong_FileDescriptor_Converter() converter stays as a fallback
when PyObject_AsFileDescriptor() cannot be used.
vstinner added a commit to vstinner/cpython that referenced this issue Mar 20, 2024
…thon#116769)

Add tests on the "fildes" converter to _testclinic_limited.
vstinner added a commit to vstinner/cpython that referenced this issue Mar 20, 2024
…6793)

Only add includes when the converter is effectively used.
vstinner added a commit to vstinner/cpython that referenced this issue Mar 20, 2024
…python#116806)

The fildes converter of Argument Clinic now always call
PyObject_AsFileDescriptor(), not only for the limited C API.

The _PyLong_FileDescriptor_Converter() converter stays as a fallback
when PyObject_AsFileDescriptor() cannot be used.
adorilson pushed a commit to adorilson/cpython that referenced this issue Mar 25, 2024
…thon#116769)

Add tests on the "fildes" converter to _testclinic_limited.
adorilson pushed a commit to adorilson/cpython that referenced this issue Mar 25, 2024
…6793)

Only add includes when the converter is effectively used.
adorilson pushed a commit to adorilson/cpython that referenced this issue Mar 25, 2024
…python#116806)

The fildes converter of Argument Clinic now always call
PyObject_AsFileDescriptor(), not only for the limited C API.

The _PyLong_FileDescriptor_Converter() converter stays as a fallback
when PyObject_AsFileDescriptor() cannot be used.
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
…thon#116769)

Add tests on the "fildes" converter to _testclinic_limited.
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
…6793)

Only add includes when the converter is effectively used.
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
…python#116806)

The fildes converter of Argument Clinic now always call
PyObject_AsFileDescriptor(), not only for the limited C API.

The _PyLong_FileDescriptor_Converter() converter stays as a fallback
when PyObject_AsFileDescriptor() cannot be used.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-C-API type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

2 participants