homepage

The Python C API manual page for data marshaling contains the following paragraph.
XXX What about error detection? It appears that reading past the end of the file will always result in a negative numeric value (where that’s relevant), but it’s not clear that negative values won’t be handled properly when there’s no error. What’s the right way to tell? Should only non-negative values be written using these routines?
I suggest that the XXX should be removed as it is unclear why it's there.
Patch to follow in the next couple of days if others agree.

It should be removed if someone is confident that it’s a obsolete comment, or if tests get added to answer the questions in the note.

Would you just remove the "XXX" string, or the entire comment? "XXX" is typically used to indicate that something needs to be done, and the comment makes a clear statement as to what it is that needs to be done.

From reading the source of Python/marshal.c it seems that the read function's raise an exception on I/O errors, but don't return a specific value (that is, sentence starting with "It appears that" is wrong). 
PyMarshal_ReadLongFromFile calls r_long, this calls r_string without checking for errors and calculates the return value from the buffer passed to r_string. On I/O errors the buffer may not have been filled at all and contains random data (whatever happened to be on the stack).
Likewise for PyMarhal_ReadShortFromFile (through r_short instead of r_long).
r_string does raise an exception on I/O errors or short reads, but reading from FILE* and Python objects.
The most straightforward documentation update would be:
* Remove the entire XXX paragraph
* Add text to the documentation for PyMarshal_ReadLongFromFile and PyMarshal_ReadShortFromFile: On error sets the appopriate exception (:exc:`EOFError`), but does not return a specific value. Use :func:`PyErr_Occurred` to check for errors.

Since 4059e871e74e, PyMarshal_ReadLongFromFile and PyMarshal_ReadShortFromFile can return -1 on error. Return values of those functions were already documented in acb4d43955f6. Some of the usages also check return value of PyErr_Occurred(): https://hg.python.org/cpython/rev/11958c69a4b2#l2.7
I removed the outdated paragraph and add a sentence about using PyErr_Occurred().

New changeset defcffdf86780e3a184ebb25dc9a7b807753d57a by Berker Peksag in branch 'master':
bpo-12743: Delete comment from marshal.rst (GH-8457)
https://github.com/python/cpython/commit/defcffdf86780e3a184ebb25dc9a7b807753d57a

New changeset 21ed29ac290b10d31dcac947f9246ae4d8b94a86 by Miss Islington (bot) in branch '3.7':
bpo-12743: Delete comment from marshal.rst (GH-8457)
https://github.com/python/cpython/commit/21ed29ac290b10d31dcac947f9246ae4d8b94a86

New changeset 146ba436cc0457b8ef7fea8b054b9ccb15e24748 by Miss Islington (bot) in branch '3.6':
bpo-12743: Delete comment from marshal.rst (GH-8457)
https://github.com/python/cpython/commit/146ba436cc0457b8ef7fea8b054b9ccb15e24748