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.

classification
Title: mmap and exception type
Type: behavior Stage: patch review
Components: Versions: Python 3.3
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: neologix, ocean-city, rhettinger, vstinner
Priority: low Keywords: needs review, patch

Created on 2009-02-27 17:23 by ocean-city, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
mmap_exceptions.diff neologix, 2011-06-02 10:07 review
Messages (5)
msg82849 - (view) Author: Hirokazu Yamamoto (ocean-city) * (Python committer) Date: 2009-02-27 17:23
It seems mmap module is using inappropriate exception types. For example,

	if (! (PyString_Check(v)) ) {
		PyErr_SetString(PyExc_IndexError,
				"mmap slice assignment must be a string");
		return -1;
	}

I think this should be PyExc_TypeError.

		if (self->size >= pos && count > self->size - pos) {
			PyErr_SetString(PyExc_ValueError,
					"source or destination out of range");
			return NULL;

I think this is out of range, so PyExc_IndexError.

Of course, there is the case difficult to determine which exception is
suitable. For example, if Py_ssize_t is negative value, OverflowError or
IndexError?
msg109758 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-07-09 16:01
Could someone please advise on the appropriate exception types for mmap.
msg137471 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-06-02 10:07
Here's a patch.
msg137489 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2011-06-02 18:11
It is uncomfortable changing exception types have been a published API for over a decade, so I'll leave this one open for a bit so that others can comment.

Most of the changes in the patch are correct and match their counterparts for lists and strings.  The TypeError for slice deletion was correct as-is (it matches del 'abc'[1]).  The IndexError on slice assignment may be correct as-is (I'll do more research).  The from ValueError to SystemError is questionable.  Otherwise, the patch looks to be mostly correct (I'll give it a more thorough review later).
msg281539 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-11-23 06:57
Though the changes look like they would nicely harmonize mmap with other modules, I'm going to decline this patch.  No one else has stepped forward to offer agreement and it likely isn't worth the disruption that would come from breaking existing code.  Unfortunately, the time the fix-up an API is when it is proposed rather than years after the ship has already sailed.

Charles-François, thank you for the patch.
History
Date User Action Args
2022-04-11 14:56:46adminsetgithub: 49634
2016-11-23 06:57:38rhettingersetstatus: open -> closed
resolution: rejected
messages: + msg281539
2014-02-03 19:16:08BreamoreBoysetnosy: - BreamoreBoy
2011-06-02 18:11:00rhettingersetpriority: normal -> low

nosy: + rhettinger
messages: + msg137489

assignee: rhettinger
2011-06-02 10:07:48neologixsetfiles: + mmap_exceptions.diff

versions: + Python 3.3, - Python 3.1, Python 2.7
keywords: + patch, needs review
nosy: + vstinner, neologix

messages: + msg137471
stage: patch review
2010-07-09 16:01:02BreamoreBoysetnosy: + BreamoreBoy
messages: + msg109758
2009-02-27 17:23:13ocean-citycreate