pythongh-105184: add return value indicating success/failure for PyMarshal_WriteLongToFile and PyMarshal_ReadLastObjectFromFile

Author: iritkatriel

Doc/c-api/marshal.rst

@@ -19,22 +19,26 @@ unmarshalling.  Version 2 uses a binary format for floating point numbers.
 ``Py_MARSHAL_VERSION`` indicates the current file format (currently 2).
 
 
-.. c:function:: void PyMarshal_WriteLongToFile(long value, FILE *file, int version)
+.. c:function:: int PyMarshal_WriteLongToFile(long value, FILE *file, int version)
 
    Marshal a :c:expr:`long` integer, *value*, to *file*.  This will only write
    the least-significant 32 bits of *value*; regardless of the size of the
    native :c:expr:`long` type.  *version* indicates the file format.
 
-   This function can fail, in which case it sets the error indicator.
-   Use :c:func:`PyErr_Occurred` to check for that.
+   Return 0 on success. Return -1 and set an exception on error.
 
-.. c:function:: void PyMarshal_WriteObjectToFile(PyObject *value, FILE *file, int version)
+   .. versionchanged:: 3.13
+   Added return value.
+
+.. c:function:: int PyMarshal_WriteObjectToFile(PyObject *value, FILE *file, int version)
 
    Marshal a Python object, *value*, to *file*.
    *version* indicates the file format.
 
-   This function can fail, in which case it sets the error indicator.
-   Use :c:func:`PyErr_Occurred` to check for that.
+   Return 0 on success. Return -1 and set an exception on error.
+
+   .. versionchanged:: 3.13
+   Added return value.
 
 .. c:function:: PyObject* PyMarshal_WriteObjectToString(PyObject *value, int version)
 

Include/marshal.h

@@ -20,8 +20,8 @@ PyAPI_FUNC(int) PyMarshal_ReadShortFromFile(FILE *);
 PyAPI_FUNC(PyObject *) PyMarshal_ReadObjectFromFile(FILE *);
 PyAPI_FUNC(PyObject *) PyMarshal_ReadLastObjectFromFile(FILE *);
 
-PyAPI_FUNC(void) PyMarshal_WriteLongToFile(long, FILE *, int);
-PyAPI_FUNC(void) PyMarshal_WriteObjectToFile(PyObject *, FILE *, int);
+PyAPI_FUNC(int) PyMarshal_WriteLongToFile(long, FILE *, int);
+PyAPI_FUNC(int) PyMarshal_WriteObjectToFile(PyObject *, FILE *, int);
 
 #ifdef __cplusplus
 }

Modules/_testcapimodule.c

@@ -1807,7 +1807,8 @@ pymarshal_write_long_to_file(PyObject* self, PyObject *args)
         return NULL;
     }
 
-    PyMarshal_WriteLongToFile(value, fp, version);
+    int ret = PyMarshal_WriteLongToFile(value, fp, version);
+    assert(ret == 0);
     assert(!PyErr_Occurred());
 
     fclose(fp);
@@ -1832,7 +1833,8 @@ pymarshal_write_object_to_file(PyObject* self, PyObject *args)
         return NULL;
     }
 
-    PyMarshal_WriteObjectToFile(obj, fp, version);
+    int ret = PyMarshal_WriteObjectToFile(obj, fp, version);
+    assert(ret == 0);
     assert(!PyErr_Occurred());
 
     fclose(fp);

Python/marshal.c

@@ -625,11 +625,7 @@ w_clear_refs(WFILE *wf)
 }
 
 /* version currently has no effect for writing ints. */
-/* Note that while the documentation states that this function
- * can error, currently it never does. Setting an exception in
- * this function should be regarded as an API-breaking change.
- */
-void
+int
 PyMarshal_WriteLongToFile(long x, FILE *fp, int version)
 {
     char buf[4];
@@ -642,28 +638,37 @@ PyMarshal_WriteLongToFile(long x, FILE *fp, int version)
     wf.version = version;
     w_long(x, &wf);
     w_flush(&wf);
+
+    /* While the documentation states that this function can error,
+     * currently it never does. Setting an exception in this function
+     * should be regarded as an API-breaking change.
+     */
+    assert(!PyErr_Occurred());
+
+    return 0;
 }
 
-void
+int
 PyMarshal_WriteObjectToFile(PyObject *x, FILE *fp, int version)
 {
     char buf[BUFSIZ];
     WFILE wf;
     if (PySys_Audit("marshal.dumps", "Oi", x, version) < 0) {
-        return; /* caller must check PyErr_Occurred() */
+        return -1;
     }
     memset(&wf, 0, sizeof(wf));
     wf.fp = fp;
     wf.ptr = wf.buf = buf;
     wf.end = wf.ptr + sizeof(buf);
     wf.error = WFERR_OK;
     wf.version = version;
-    if (w_init_refs(&wf, version)) {
-        return; /* caller must check PyErr_Occurred() */
+    if (w_init_refs(&wf, version) < 0) {
+        return -1;
     }
     w_object(x, &wf);
     w_clear_refs(&wf);
     w_flush(&wf);
+    return 0;
 }
 
 typedef struct {
@@ -1612,6 +1617,7 @@ PyMarshal_ReadObjectFromFile(FILE *fp)
     Py_DECREF(rf.refs);
     if (rf.buf != NULL)
         PyMem_Free(rf.buf);
+    assert((result != NULL) == !PyErr_Occurred());
     return result;
 }
 
@@ -1652,7 +1658,7 @@ PyMarshal_WriteObjectToString(PyObject *x, int version)
     wf.end = wf.ptr + PyBytes_GET_SIZE(wf.str);
     wf.error = WFERR_OK;
     wf.version = version;
-    if (w_init_refs(&wf, version)) {
+    if (w_init_refs(&wf, version) < 0) {
         Py_DECREF(wf.str);
         return NULL;
     }