diff --git a/HISTORY.rst b/HISTORY.rst index acd3fca..a51491c 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -2,6 +2,22 @@ History ===================== +0.3.1rc1 (TODO) +===================== + +- TODO. + +0.3.1rc0 (2025-04-01) +===================== + +- Add a single index entry. This is documentation for 0.3.1rc0, 341 pages. +- Add link to compiler flags resources. +- Add explicit #include that was failing on some platforms. +- Include note on ml_flags failure on import. +- Suggest change for Py_SETREF(). +- Add warning on Py_SETREF and Py_XSETREF. Fixes to Python 3 links. +- Fix issue #33: Sections in the "Parsing Python Arguments" chapter are of the wrong depth. + 0.3.0 (2025-03-20) ===================== diff --git a/doc/sphinx/PyExtPatt_0.3.1rc0.pdf b/doc/sphinx/PyExtPatt_0.3.1rc0.pdf new file mode 100644 index 0000000..dc175c7 Binary files /dev/null and b/doc/sphinx/PyExtPatt_0.3.1rc0.pdf differ diff --git a/doc/sphinx/source/compiler_flags.rst b/doc/sphinx/source/compiler_flags.rst index 7888242..ded4b97 100644 --- a/doc/sphinx/source/compiler_flags.rst +++ b/doc/sphinx/source/compiler_flags.rst @@ -239,3 +239,14 @@ This looks something like this (wrapped for clarity and replaced user with `_ + diff --git a/doc/sphinx/source/conf.py b/doc/sphinx/source/conf.py index 64f7103..a5f1a45 100644 --- a/doc/sphinx/source/conf.py +++ b/doc/sphinx/source/conf.py @@ -57,7 +57,7 @@ # The short X.Y version. version = '0.3' # The full version, including alpha/beta/rc tags. -release = '0.3.0' +release = '0.3.1rc1' todo_include_todos = True todo_link_only = True @@ -189,14 +189,16 @@ # -- Options for LaTeX output --------------------------------------------- latex_elements = { -# The paper size ('letterpaper' or 'a4paper'). -#'papersize': 'letterpaper', + # The paper size ('letterpaper' or 'a4paper'). + # Default is 'letterpaper' + 'papersize': 'a4paper', -# The font size ('10pt', '11pt' or '12pt'). -#'pointsize': '10pt', + # The font size ('10pt', '11pt' or '12pt'). + # Default is '10pt' + #'pointsize': '10pt', -# Additional stuff for the LaTeX preamble. -#'preamble': '', + # Additional stuff for the LaTeX preamble. + #'preamble': '', 'preamble': r'''\usepackage{lscape}''', } diff --git a/doc/sphinx/source/containers_and_refcounts.rst b/doc/sphinx/source/containers_and_refcounts.rst index 5703b14..3e1a8bf 100644 --- a/doc/sphinx/source/containers_and_refcounts.rst +++ b/doc/sphinx/source/containers_and_refcounts.rst @@ -298,6 +298,8 @@ Lets see: /* Now value_b ref count will remain 1 and value_a ref count will have been decremented * In this case value_a will have been free'd. */ +So `PyTuple_SetItem()`_ *discards* (see :ref:`chapter_containers_and_refcounts.discarded`) the original reference. + .. warning:: What happens if you use `PyTuple_SetItem()`_ to replace a value with the *same* value? @@ -334,7 +336,9 @@ Lets see: A simple change to `PyTuple_SetItem()`_ would prevent this from producing undefined behaviour by checking if the replacement is the same as the existing value. - `PyTuple_SET_ITEM()`_ does not exhibit this problem as it *abandons* values rather than *discarding* them. + `PyTuple_SET_ITEM()`_ does not exhibit this problem as it *abandons* + (see :ref:`chapter_containers_and_refcounts.abandoned`) values rather than *discarding* + (see :ref:`chapter_containers_and_refcounts.discarded`) them. For code and tests see: @@ -1865,7 +1869,7 @@ Tuples - On failure `PyTuple_SetItem()`_ will decrement the reference count of the given value and this might well lead to a SIGSEGV. See :ref:`chapter_containers_and_refcounts.tuples.PyTuple_SetItem.failures`. -- `PyTuple_SET_ITEM()`_ will lead to a memory leak when replacing an existing value as it abandons the previous +- `PyTuple_SET_ITEM()`_ will lead to a memory leak when replacing an existing value as it *abandons* the previous reference. See :ref:`chapter_containers_and_refcounts.tuples.PyTuple_SET_ITEM`. - `PyTuple_SET_ITEM()`_ does no error checking so is capable of writing to arbitrary memory locations on error. @@ -1896,7 +1900,7 @@ Lists - On failure `PyList_SetItem()`_ will decrement the reference count of the given value and this might well lead to a SIGSEGV. See :ref:`chapter_containers_and_refcounts.lists.PyList_SET_ITEM.failures`. -- `PyList_SET_ITEM()`_ will lead to a memory leak when replacing an existing value as it abandons the previous +- `PyList_SET_ITEM()`_ will lead to a memory leak when replacing an existing value as it *abandons* the previous reference. See :ref:`chapter_containers_and_refcounts.lists.PyList_SET_ITEM`. - `PyList_SET_ITEM()`_ does no error checking so is capable of writing to arbitrary memory locations on error. diff --git a/doc/sphinx/source/exceptions.rst b/doc/sphinx/source/exceptions.rst index 604f757..7e52879 100644 --- a/doc/sphinx/source/exceptions.rst +++ b/doc/sphinx/source/exceptions.rst @@ -27,7 +27,7 @@ These CPython calls are the most useful: * ``PyErr_Clear()`` - Clearing any set exceptions, have good reason to do this! `PyErr_Clear() documentation `_ * ``PyErr_Print()`` - Print a representation of the current exception then clear any set exceptions. - `PyErr_Print() documentation `_ + `PyErr_Print() documentation `_ Indicating an error condition is a two stage process; your code must register an exception and then indicate failure by returning ``NULL``. Here is a C function doing just that: diff --git a/doc/sphinx/source/parsing_arguments.rst b/doc/sphinx/source/parsing_arguments.rst index 313559c..dffb2af 100644 --- a/doc/sphinx/source/parsing_arguments.rst +++ b/doc/sphinx/source/parsing_arguments.rst @@ -55,15 +55,18 @@ supress a compiler warning or error thus: parse_args_kwargs(PyObject *Py_UNUSED(module), PyObject *args, PyObject *kwargs); .. index:: + single: ml_flags single: Parsing Arguments; ml_flags +------------------------------ Setting the ``ml_flags`` Field ------------------------------ -The `ml_flags `_ field in -`PyMethodDef `_ specifies the form of the arguments. +The `ml_flags `_ field in +`PyMethodDef `_ specifies the form of the arguments. .. index:: + single: METH_NOARGS single: Parsing Arguments; No Arguments single: Parsing Arguments; METH_NOARGS @@ -75,6 +78,7 @@ No Arguments - The second argument will be ``NULL``. .. index:: + single: METH_O single: Parsing Arguments; One Argument single: Parsing Arguments; METH_O @@ -86,6 +90,7 @@ One Argument - The second argument will be the single argument. .. index:: + single: METH_VARARGS single: Parsing Arguments; Multiple Arguments single: Parsing Arguments; METH_VARARGS @@ -98,6 +103,7 @@ Multiple Positional Arguments - `PyArg_ParseTuple()`_ is used to unpack the arguments. .. index:: + single: METH_KEYWORDS single: Parsing Arguments; Positional and Keyword Arguments single: Parsing Arguments; METH_KEYWORDS @@ -151,8 +157,41 @@ And this would be added to the module, say, by using: {NULL, NULL, 0, NULL} /* Sentinel */ }; +Failure +^^^^^^^^ + +If the ml_flags are in conflict then a ``SystemError`` will be raised on import. + +For example, the conflict with ``METH_NOARGS | METH_KEYWORDS``: + +.. code-block:: c + + static PyMethodDef cParseArgs_methods[] = { + /* ... */ + { + "clear", + (PyCFunction) clear, + METH_NOARGS | METH_KEYWORDS, + "Documentation for clear()." + }, + /* ... */ + {NULL, NULL, 0, NULL} /* Sentinel */ + }; + +On import this gives: + +.. code-block:: python + + >>> import cParseArgs + Traceback (most recent call last): + File "", line 1, in + SystemError: clear() method: bad call flags + +Which identifies the problematic function but not the actual problem. + +===================== Parsing the Arguments ------------------------------- +===================== Once we have the C function correctly declared then the arguments have to parsed according to their types and, if required, converted to C types (so-called "unboxing"). @@ -196,15 +235,18 @@ The reference documentation is excellent: `argument parsing and building values without specifying which argument it is referring to. +==================================== Examples ==================================== These examples are in ``src/cpy/cParseArgs.c`` and their tests are in ``tests/unit/test_c_parse_args.py``. .. index:: + single: METH_NOARGS single: Parsing Arguments Example; No Arguments single: Parsing Arguments; METH_NOARGS +------------------------------------ No Arguments ------------------------------------ @@ -237,9 +279,11 @@ The Python interpreter will raise a ``TypeError`` on any arguments are offered t }; .. index:: + single: METH_O single: Parsing Arguments Example; One Argument single: Parsing Arguments; METH_O +------------------------------------ One Argument ------------------------------------ @@ -323,9 +367,11 @@ So it is best to fail fast, an near the error site, that dastardly Side note: Of course this does not protect you from malicious/badly written code that decrements by more than one :-) .. index:: + single: METH_VARARGS single: Parsing Arguments Example; Variable Number of Arguments single: Parsing Arguments; METH_VARARGS +------------------------------------ Variable Number of Arguments ---------------------------------------------------- @@ -381,10 +427,12 @@ Failure modes, when the wrong arguments are passed are tested in ``tests.unit.te Note the wide variety of error messages that are obtained. .. index:: + single: METH_VARARGS + single: METH_KEYWORDS single: Parsing Arguments Example; Variable Number of Arguments single: Parsing Arguments Example; Keyword Arguments - single: Parsing Arguments; METH_KEYWORDS +-------------------------------------------------------------------------- Variable Number of Arguments and Keyword Arguments -------------------------------------------------------------------------- @@ -439,7 +487,7 @@ Here is the C code, note the string ``"O|i"`` that describes the argument types return ret; } -This function can be added to the module with both the ``METH_VARARGS`` and ``METH_KEYWORDS`` flags set: +This function must be added to the module with both the ``METH_VARARGS`` and ``METH_KEYWORDS`` flags set: .. code-block:: c @@ -498,6 +546,7 @@ The solution is to cast away const in the call: single: Default Arguments; C single: Py_buffer +------------------------------------------ Default String and Bytes Arguments ------------------------------------------ @@ -570,6 +619,7 @@ See ``tests/unit/test_c_parse_args.py`` for some Python uses of this code. single: Parsing Arguments Example; Positional Only Arguments single: Parsing Arguments Example; Keyword Only Arguments +----------------------------------------------- Positional Only and Keyword Only Arguments ----------------------------------------------- @@ -637,6 +687,7 @@ Here is the C code: .. index:: single: Parsing Arguments Example; Functional Conversion to C +--------------------------------------------------------- Parsing Arguments With Functional Conversion to C --------------------------------------------------------- @@ -719,6 +770,7 @@ Here is the C code. .. _cpython_default_mutable_arguments: +============================================= Being Pythonic with Default Mutable Arguments ============================================= @@ -928,6 +980,7 @@ Tests are in ``test_parse_args_with_mutable_defaults()`` in ``tests/unit/test_c_ .. index:: single: Parsing Arguments Example; Helper Macros +============= Helper Macros ============= @@ -982,6 +1035,7 @@ And a macro to check the type of the argument: .. index:: single: Default Arguments, Immutable; C +------------------- Immutable Arguments ------------------- @@ -1056,6 +1110,7 @@ If you are in a C++ environment then the section on :ref:`cpp_and_cpython.handli .. index:: single: Default Arguments, Mutable; C +------------------- Mutable Arguments ------------------- diff --git a/doc/sphinx/source/refcount.rst b/doc/sphinx/source/refcount.rst index 37cf2af..c15f634 100644 --- a/doc/sphinx/source/refcount.rst +++ b/doc/sphinx/source/refcount.rst @@ -5,13 +5,16 @@ :maxdepth: 3 .. - .. _Reference Counting: https://docs.python.org/3.9/c-api/refcounting.html + .. _Reference Counting: https://docs.python.org/3/c-api/refcounting.html + +.. _Py_REFCNT(): https://docs.python.org/3/c-api/structures.html#c.Py_REFCNT +.. _Py_INCREF(): https://docs.python.org/3/c-api/refcounting.html#c.Py_INCREF +.. _Py_XINCREF(): https://docs.python.org/3/c-api/refcounting.html#c.Py_XINCREF +.. _Py_DECREF(): https://docs.python.org/3/c-api/refcounting.html#c.Py_DECREF +.. _Py_XDECREF(): https://docs.python.org/3/c-api/refcounting.html#c.Py_XDECREF +.. _Py_SETREF(): https://docs.python.org/3/c-api/refcounting.html#c.Py_SETREF +.. _Py_XSETREF(): https://docs.python.org/3/c-api/refcounting.html#c.Py_XSETREF -.. _Py_REFCNT(): https://docs.python.org/3.9/c-api/structures.html#c.Py_REFCNT -.. _Py_INCREF(): https://docs.python.org/3.9/c-api/refcounting.html#c.Py_INCREF -.. _Py_XINCREF(): https://docs.python.org/3.9/c-api/refcounting.html#c.Py_XINCREF -.. _Py_DECREF(): https://docs.python.org/3.9/c-api/refcounting.html#c.Py_DECREF -.. _Py_XDECREF(): https://docs.python.org/3.9/c-api/refcounting.html#c.Py_XDECREF .. _chapter_refcount: @@ -75,6 +78,17 @@ The macros to manipulate reference counts are: count could be anything. * - `Py_XDECREF()`_ - As `Py_DECREF()`_ but does nothing if passed ``NULL``. + * - `Py_SETREF()`_ + - This decrements the first arguments reference count then replaces it with the second argument. + Thus it gracefully replaces one ``PyObject *`` with another. + + .. warning:: + See :ref:`chapter_refcount.warning_Py_SETREF()_Py_XSETREF`. + * - `Py_XSETREF()`_ + - As `Py_SETREF()`_ but if passed ``NULL`` as the first argument replaces the first argument with the second. + + .. warning:: + See :ref:`chapter_refcount.warning_Py_SETREF()_Py_XSETREF`. Here is an example of a normal ``PyObject`` creation, print and de-allocation: @@ -201,6 +215,68 @@ And here is what happens to the memory if we use this function from Python (``cP +.. index:: + single: Py_SETREF() + pair: Documentation Lacunae; Py_SETREF() + single: Py_XSETREF() + pair: Documentation Lacunae; Py_XSETREF() + +.. _chapter_refcount.warning_Py_SETREF()_Py_XSETREF: + +------------------------------------------------------- +Warning About ``Py_SETREF()`` and ``Py_XSETREF()`` +------------------------------------------------------- + +.. warning:: + + `Py_SETREF()`_ and `Py_XSETREF()`_ contain a flaw. + The idea behind them is to 'swap' the arguments, decrementing the first argument's reference count. + + The implementation (for `Py_SETREF()`_) looks like this: + + .. code-block:: c + + #define Py_SETREF(op, op2) \ + do { \ + PyObject *_py_tmp = _PyObject_CAST(op); \ + (op) = (op2); \ + Py_DECREF(_py_tmp); \ + } while (0) + + However if called with both arguments pointing to the same ``PyObject *`` *and* that has a reference count of unity + this will deallocate the object whilst maintaining a reference to it. + Then a segfault is certain to happen later on as soon the de-referenced ``PyObject *`` is accessed. + + The greater the first argument's reference count is, then the further away from the call to `Py_SETREF()`_ the + segfault will happen. + + For example, in the simple case: + + .. code-block:: c + + PyObject *op = PyUnicode_FromString("My test string."); + assert(Py_REFCNT(op) == 1); + Py_SETREF(op, op); + /* This is now undefined as it is access after free + * and will most likely a segfault. */ + Py_DECREF(op); + + This issue is at the heart of the problem described in + :ref:`chapter_containers_and_refcounts.tuples.PyTuple_SetItem.replacement`. + + A simple change to these macros would eliminate this problem: + + .. code-block:: c + + #define Py_SETREF(op, op2) \ + do { \ + if (op != op2) { \ + PyObject *_py_tmp = _PyObject_CAST(op); \ + (op) = (op2); \ + Py_DECREF(_py_tmp); \ + } \ + } while (0) + .. _chapter_refcount.warning_ref_count_unity: ------------------------------------------------------- @@ -451,7 +527,7 @@ Warning on "Stolen" References With Containers will increment them correctly. Unfortunately this was only made clear in the Python documentation for ``PyDict_SetItem`` in Python version 3.8+: - https://docs.python.org/3.8/c-api/dict.html + https://docs.python.org/3/c-api/dict.html This also happens with `Py_BuildValue `_ when building with newly created Python objects (using ``Py_BuildValue("{OO}", ...``). @@ -557,7 +633,7 @@ At least this will get your attention! .. note:: Incidentally from Python 3.3 onwards there is a module - `faulthandler `_ + `faulthandler `_ that can give useful debugging information (file ``FaultHandlerExample.py``): .. code-block:: python diff --git a/setup.py b/setup.py index 078bacd..d5644c4 100644 --- a/setup.py +++ b/setup.py @@ -319,7 +319,7 @@ # For keywords see: https://setuptools.pypa.io/en/latest/references/keywords.html setup( name=PACKAGE_NAME, - version='0.3.0', + version='0.3.1rc1', author='Paul Ross', author_email='apaulross@gmail.com', maintainer='Paul Ross', diff --git a/src/cpy/File/PythonFileWrapper.cpp b/src/cpy/File/PythonFileWrapper.cpp index 100b908..94b2595 100644 --- a/src/cpy/File/PythonFileWrapper.cpp +++ b/src/cpy/File/PythonFileWrapper.cpp @@ -5,6 +5,7 @@ #include "PythonFileWrapper.h" #include +#include /** * Macro that gets the given method and checks that it is callable. @@ -13,7 +14,7 @@ #define EXTRACT_METHOD_AND_CHECK(name) \ m_python_##name##_method = PyObject_GetAttrString(python_file_object, #name); /* New ref. */\ if (!m_python_##name##_method) { \ - std::ostringstream oss; \ + std::ostringstream oss; \ oss << "PythonFileObjectWrapper: can not get method: " << #name << std::endl; \ Py_XDECREF(python_file_object); \ Py_XDECREF(m_python_read_method); \ @@ -23,7 +24,7 @@ throw ExceptionPythonFileObjectWrapper(oss.str()); \ } \ if (!PyCallable_Check(m_python_##name##_method)) { \ - std::ostringstream oss; \ + std::ostringstream oss; \ oss << "PythonFileObjectWrapper: method: " << #name << " is not callable" << std::endl; \ Py_XDECREF(m_python_file_object); \ Py_XDECREF(m_python_read_method); \