[issue12743] C API marshalling doc contains XXX

[Berker Peksag]

Change by Berker Peksag :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed
versions: +Python 3.8 -Python 3.5

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12743] C API marshalling doc contains XXX

[miss-islington]

miss-islington  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12743] C API marshalling doc contains XXX

[miss-islington]

miss-islington  added the comment:


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


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12743] C API marshalling doc contains XXX

[miss-islington]

Change by miss-islington :


--
pull_requests: +8010

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12743] C API marshalling doc contains XXX

[miss-islington]

Change by miss-islington :


--
pull_requests: +8011

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12743] C API marshalling doc contains XXX

[Berker Peksag]

Berker Peksag  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12743] C API marshalling doc contains XXX

[Berker Peksag]

Change by Berker Peksag :


--
pull_requests: +7981

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12743] C API marshalling doc contains XXX

[Berker Peksag]

Changes by Berker Peksag :


--
versions: +Python 3.7
Added file: http://bugs.python.org/file44871/issue12743_v2.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12743] C API marshalling doc contains XXX

[Berker Peksag]

Berker Peksag added the comment:

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().

--
keywords: +patch
nosy: +berker.peksag, haypo
stage: needs patch -> patch review
versions: +Python 3.5, Python 3.6
Added file: http://bugs.python.org/file42555/issue12743.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12743] C API marshalling doc contains XXX

[Serhiy Storchaka]

Changes by Serhiy Storchaka storch...@gmail.com:


--
stage:  - needs patch
type:  - behavior

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12743
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12743] C API marshalling doc contains XXX

[Ronald Oussoren]

Ronald Oussoren added the comment:

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.

--
nosy: +ronaldoussoren

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12743
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12743] C API marshalling doc contains XXX

[Martin v . Löwis]

Martin v. Löwis mar...@v.loewis.de added the comment:

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.

--
nosy: +loewis

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12743
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12743] C API marshalling doc contains XXX

[JJeffries]

New submission from JJeffries jamesjeffri...@gmail.com:

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.

--
assignee: docs@python
components: Documentation
messages: 141959
nosy: JJeffries, docs@python
priority: normal
severity: normal
status: open
title: C API marshalling doc contains XXX

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12743
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12743] C API marshalling doc contains XXX

[Éric Araujo]

Éric Araujo mer...@netwok.org added the comment:

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.

--
nosy: +eric.araujo

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12743
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com