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

gh-85283: Add PySys_Audit() to the limited C API #108571

Merged
merged 1 commit into from
Oct 17, 2023

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Aug 28, 2023

This function was added in Python 3.8 by the PEP 578 "Python Runtime Audit Hooks". It is needed to convert some stdlib extensions to the limited C API, like fcntl, resource and syslog.

Move also non-limited "PerfMap" C API from Include/sysmodule.h to Include/cpython/sysmodule.h.


📚 Documentation preview 📚: https://cpython-previews--108571.org.readthedocs.build/

@encukou
Copy link
Member

encukou commented Aug 31, 2023

IMO this function is a poor fit for the limited API: it uses varargs, making it difficult to use from non-C languages. See also capi-workgroup/problems#35

@vstinner
Copy link
Member Author

IMO this function is a poor fit for the limited API: it uses varargs, making it difficult to use from non-C languages. See also capi-workgroup/problems#35

The C API has multiple functions using varargs:

  • PyErr_Format()
  • PyUnicode_FromFormat()

Right, they cannot be used in programming languages other than C.

But well, C remains widely used, and using this function is relevant in C, no? Which alternative do you propose?

PySys_Audit() is related to security and it doesn't get the sys.audit() function from the sys module but uses directly the C implementation. Overriding sys.audit() has no effect on PySys_Audit().

@encukou
Copy link
Member

encukou commented Sep 4, 2023

The C API has multiple functions using varargs

Yup, but they all have alternatives that don't use varargs -- you can use PyErr_SetString or PyUnicode_FromStringAndSize, after you format the string using some other mechanism.

There's no such alternative to PySys_Audit. IMO, one should be added before this is added to stable ABI. Perhaps a function that takes the tuple of arguments (which is slower as the tuple would always be built, so PySys_Audit would still be preferred if you can use varargs).

@vstinner
Copy link
Member Author

vstinner commented Sep 4, 2023

@encukou:

Yup, but they all have alternatives that don't use varargs -- you can use PyErr_SetString or PyUnicode_FromStringAndSize, after you format the string using some other mechanism.
There's no such alternative to PySys_Audit. IMO, one should be added before this is added to stable ABI. Perhaps a function that takes the tuple of arguments (which is slower as the tuple would always be built, so PySys_Audit would still be preferred if you can use varargs).

I'm not sure that I can follow your rationale. Because there are programming languages which cannot use a C API with variadic arguments, the ... arguments in PySys_Audit(), you would like that all users of the limited C API have to go through a slower implementation, by always creating the tuple?

These programming languages can already build a tuple and call sys.audit(). There is no need to add anything here. For me, the most convenient is PyTuple_Pack(), but it also uses variadic arguments: PyObject* PyTuple_Pack(Py_ssize_t, ...). Or you can use PyObject* Py_BuildValue(const char *, ...), oh wait, it also uses variadic arguments.

Are you sure that there are programming languages which are used to write Python extensions and cannot use variadic arguments? In C, variadic arguments are very commonly used, by printf() for example.

For me, the advantage of adding PySys_Audit() is that its API is easier to use than having to build a tuple manually (for check errors), get the sys module (check for errors), call the audit() function (check for errors), and clear temporary objects.

Note: PySys_Audit() is implemented by calling Py_VaBuildValue(argFormat, vargs) to make a Python tuple object.


If there are programming languages which are used to write Python extensions and cannot use variadic arguments, why not letting them to call sys.audit() via the "Python" API (create a tuple, get the sys module, call the audit() function)? Why would other programming languages have to suffer from that?

I think that C and C++ are still commonly used these days to write C extensions for Python.

@encukou
Copy link
Member

encukou commented Sep 4, 2023

I'm not sure that I can follow your rationale. Because there are programming languages which cannot use a C API with variadic arguments, the ... arguments in PySys_Audit(), you would like that all users of the limited C API have to go through a slower implementation, by always creating the tuple?

No. Those that can use PySys_Audit should still use that. I'm asking to also add an alternative.

Are you sure that there are programming languages which are used to write Python extensions and cannot use variadic arguments?

It was mentioned by @steve-s in capi-workgroup/problems#35 (comment). Should we dig for the rationale?

In C, variadic arguments are very commonly used, by printf() for example.

Yup, C uses them a lot, but e.g. Rust is doing just fine without vararg functions.

These programming languages can already build a tuple and call sys.audit().

As you mentioned, sys.audit can be reassigned by the user. It's not a proper replacement.

@vstinner
Copy link
Member Author

vstinner commented Sep 5, 2023

I wrote PR #108965 to add PySys_AuditTuple() to the non-limited C API.

Since it's a new API, I would prefer to start by adding it the non-limited C API first, and wait one version to move it the limited C API. Just in case if something goes wrong.

@encukou
Copy link
Member

encukou commented Sep 7, 2023

Thank you!
OK, but let's wait with PySys_Audit too, so both are added at the same time.

@vstinner
Copy link
Member Author

OK, but let's wait with PySys_Audit too, so both are added at the same time.

Sorry, you want to wait for what?

@encukou
Copy link
Member

encukou commented Sep 18, 2023

I would prefer to wait until both PySys_Audit and PySys_AuditTuple can be added to the stable ABI, together.

This function was added in Python 3.8 by the PEP 578 "Python Runtime
Audit Hooks". Add also PySys_AuditTuple() to the limited C API,
function added to Python 3.13.

Move also non-limited "PerfMap" C API from Include/sysmodule.h to
Include/cpython/sysmodule.h.
@vstinner
Copy link
Member Author

I would prefer to wait until both PySys_Audit and PySys_AuditTuple can be added to the stable ABI, together.

Right. I updated my PR to add PySys_Audit() and PySys_AuditTuple() to the limited C API version 3.13 (and so to the stable ABI).

@vstinner vstinner merged commit 2324652 into python:main Oct 17, 2023
27 checks passed
@vstinner vstinner deleted the limited_sys_audit branch October 17, 2023 14:02
@vstinner
Copy link
Member Author

Merged. Thanks @encukou for telling me about variadic arguments, the additional of the PySys_AuditTuple() function solves this issue.

aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
The PySys_Audit() function was added in Python 3.8 by the PEP 578
"Python Runtime Audit Hooks".

Add also PySys_AuditTuple() to the limited C API, function added
to Python 3.13.

Move non-limited "PerfMap" C API from Include/sysmodule.h to
Include/cpython/sysmodule.h.
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
The PySys_Audit() function was added in Python 3.8 by the PEP 578
"Python Runtime Audit Hooks".

Add also PySys_AuditTuple() to the limited C API, function added
to Python 3.13.

Move non-limited "PerfMap" C API from Include/sysmodule.h to
Include/cpython/sysmodule.h.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants