This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author larry
Recipients Matt.Mackall, benjamin.peterson, brett.cannon, eric.smith, ezio.melotti, josh.r, larry, loewis, ncoghlan, pitrou, serhiy.storchaka, vstinner
Date 2016-05-15.16:04:27
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1463328268.09.0.867307643648.issue21199@psf.upfronthosting.co.za>
In-reply-to
Content
Martin said:

> Josh: it's not as simple as just changing the type code and variable type. Systems used to require all kinds of types in the length parameter of read(2) and fread(3), including int, long, and size_t. If it was int, passing size_t would lead to silent truncation. So at a minimum, a careful review of the further processing, and a test case is necessary.

file_read() calls Py_UniversalNewlineFread() to do the actual reading.  The length argument of Py_UniversalNewlineFread(), "n", is already of type size_t, and it passes this parameter in to fread() for the number-of-elements argument.  Py_UniversalNewlineFread() is also used by file_readinto(), and that function already passes in a size_t.  So it's already successfully being used in 2.7.

If Python 2.7 still supports systems where fread()'s number-of-elements argument is smaller than size_t, this should already be causing a compilation warning on such platforms.  One would hope people supporting such platforms would fix it.  I don't know of any such platforms, so I cannot address this hypothetical issue.  Do you know of platforms supported by 2.7 where fread's number-of-elements parameter is smaller than size_t?


Antoine said:

> The benefit of fixing this in the next 2.7.x iteration is small compared to the cost of a potential regression in the core I/O implementation.

Although nobody wants to cause a regression in CPython, I'm having difficulty imagining how this change could plausibly cause one.  On the other hand, Matt makes it clear that it's a bug that has bitten hg, so I think it's worth fixing.

Can you suggest a way this change could plausibly cause a regression?
History
Date User Action Args
2016-05-15 16:04:28larrysetrecipients: + larry, loewis, brett.cannon, ncoghlan, pitrou, vstinner, eric.smith, benjamin.peterson, ezio.melotti, Matt.Mackall, serhiy.storchaka, josh.r
2016-05-15 16:04:28larrysetmessageid: <1463328268.09.0.867307643648.issue21199@psf.upfronthosting.co.za>
2016-05-15 16:04:28larrylinkissue21199 messages
2016-05-15 16:04:27larrycreate