[issue2122] mmap.flush does not check for errors on windows

[Berker Peksag]

Berker Peksag  added the comment:

Thanks for the suggestions and for the reviews!

--
assignee: brian.curtin -> 
priority: high -> normal
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

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

[issue2122] mmap.flush does not check for errors on windows

[Berker Peksag]

Berker Peksag  added the comment:


New changeset e7d4b2f205c711d056bea73a0093e2e2b200544b by Berker Peksag in 
branch 'master':
bpo-2122: Make mmap.flush() behave same on all platforms (GH-8692)
https://github.com/python/cpython/commit/e7d4b2f205c711d056bea73a0093e2e2b200544b


--
nosy: +berker.peksag

___
Python tracker 

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

[issue2122] mmap.flush does not check for errors on windows

[Steve Dower]

Steve Dower  added the comment:

I agree. We should definitely document it as a change, but I don't think 
there's anything useful you can do in the case of a flush failing anyway, so it 
seems unlikely that anyone could depend on the return value.

Returning None for success and exception on error sounds like a good change. 
Technically this is no longer returning zero, as previously documented, so if 
anyone wants to get *really* pedantic, we're not returning the "failed" result 
for success ;)

--

___
Python tracker 

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

[issue2122] mmap.flush does not check for errors on windows

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

If we break compatibility in any case, what if return None instead of 0? The 
function always returning 0 looks strange. The function always returning None 
is common.

--
nosy: +serhiy.storchaka
versions: +Python 3.8 -Python 2.7, Python 3.4, Python 3.5

___
Python tracker 

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

[issue2122] mmap.flush does not check for errors on windows

[Mark Lawrence]

Change by Mark Lawrence :


--
nosy:  -BreamoreBoy

___
Python tracker 

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

[issue2122] mmap.flush does not check for errors on windows

[Berker Peksag]

Change by Berker Peksag :


--
pull_requests: +8187

___
Python tracker 

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

[issue2122] mmap.flush does not check for errors on windows

[Brian Curtin]

Changes by Brian Curtin br...@python.org:


--
nosy:  -brian.curtin

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

[issue2122] mmap.flush does not check for errors on windows

[Mark Lawrence]

Mark Lawrence added the comment:

I think we should be properly handling errors.  If people agree I'll provide a 
new patch to cover code and doc changes, but I've no idea how to provide any 
form of unit test for the change.

--
nosy: +BreamoreBoy, paul.moore, steve.dower, zach.ware
versions: +Python 3.4, Python 3.5 -Python 3.2, Python 3.3

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

[issue2122] mmap.flush does not check for errors on windows

[Atsuo Ishimoto]

Changes by Atsuo Ishimoto ishim...@gembook.org:


--
nosy: +ishimoto

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

[issue2122] mmap.flush does not check for errors on windows

[Antoine Pitrou]

Changes by Antoine Pitrou pit...@free.fr:


--
stage: test needed - patch review

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

[issue2122] mmap.flush does not check for errors on windows

[Terry J. Reedy]

Changes by Terry J. Reedy tjre...@udel.edu:


--
versions: +Python 3.3 -Python 3.1

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

[issue2122] mmap.flush does not check for errors on windows

[Hirokazu Yamamoto]

Hirokazu Yamamoto ocean-c...@m2.ccsnet.ne.jp added the comment:

This issue seems to be reproduced in following way.

1. Attach USB flash drive. (On my machine, it was attached as E drive)

2. Run python interactive shell and run following commands.
   (Confirmed on Python2.6)

   import mmap
   f = open(e:/temp.tmp, w+b)
   f.write(foo)
   f.flush()
   m = mmap.mmap(f.fileno(), 3)
   m[:] = xxx

3. Detach USB flash drive violently here! (Without safety mechanism
   to detach USB flash drive, just plug it off) Note that
   python shell is still running.

4. Enter following command in python shell.
   m.flush()
   It returns *0*. (Means failure)

--

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

[issue2122] mmap.flush does not check for errors on windows

[Antoine Pitrou]

Changes by Antoine Pitrou pit...@free.fr:


--
nosy: +brian.curtin, tim.golden
versions:  -Python 2.6

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

[issue2122] mmap.flush does not check for errors on windows

[Brian Curtin]

Changes by Brian Curtin cur...@acm.org:


--
assignee:  - brian.curtin

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

[issue2122] mmap.flush does not check for errors on windows

[Terry J. Reedy]

Changes by Terry J. Reedy tjre...@udel.edu:


--
versions: +Python 2.7, Python 3.2

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

[issue2122] mmap.flush does not check for errors on windows

[Daniel Diniz]

Changes by Daniel Diniz aja...@gmail.com:


--
priority:  - high
stage:  - test needed
versions: +Python 3.1 -Python 2.5, Python 3.0

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

[issue2122] mmap.flush does not check for errors on windows

[Antoine Pitrou]

Changes by Antoine Pitrou pit...@free.fr:


--
nosy: +ocean-city

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

[issue2122] mmap.flush does not check for errors on windows

[Ralf Schmitt]

Ralf Schmitt [EMAIL PROTECTED] added the comment:

now this insanity is even documented with svn revision 62359
(http://hgpy.de/py/trunk/rev/442252bce780)

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2122
__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2122] mmap.flush does not check for errors on windows

[Antoine Pitrou]

Antoine Pitrou [EMAIL PROTECTED] added the comment:

Perhaps it would be better if the patch came with unit tests. I agree
that the situation right now is insane.

PS : I have no power to commit or even accept a patch :)

--
nosy: +pitrou

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2122
__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2122] mmap.flush does not check for errors on windows

[Ralf Schmitt]

Ralf Schmitt [EMAIL PROTECTED] added the comment:

I thought about this too, but I don't know of a way to provoke an error.
(Other than passing in illegal values, but the code tries to catch
those. And btw, raises an Exception on windows :)

One could currently pass in a negative value for size (this isn't caught
in the checks). e.g.

m.flush(500, -500) gives an 'error: [Errno 22] Invalid argument' on linux.

but then you don't want to rely on another bug for testing purposes.

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2122
__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2122] mmap.flush does not check for errors on windows

[Martin v. Löwis]

Changes by Martin v. Löwis [EMAIL PROTECTED]:


--
keywords: +patch

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2122
__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2122] mmap.flush does not check for errors on windows

[Ralf Schmitt]

Ralf Schmitt [EMAIL PROTECTED] added the comment:

attached is a patch. I hope it is ok to change the semantics a bit. 
They were very strange:
- on unix it raises an exception on errors. otherwise it always returns 0.
- on windows it returns non-zero on success, and returns zero on errors.

Now, flush returns None or raises an Exception.

Added file: http://bugs.python.org/file9645/mmap-flush.txt

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2122
__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue2122] mmap.flush does not check for errors on windows

[Ralf Schmitt]

New submission from Ralf Schmitt:

mmap.flush returns the result of the call to FlushViewOfFile as an
integer, and does not check for errors. On unix it does check for
errors. The function should return None and raise an exception if an
error occurs...
This bug can lead to data loss...

Here's the current version of that function:

static PyObject *
mmap_flush_method(mmap_object *self, PyObject *args)
{
Py_ssize_t offset = 0;
Py_ssize_t size = self-size;
CHECK_VALID(NULL);
if (!PyArg_ParseTuple(args, |nn:flush, offset, size))
return NULL;
if ((size_t)(offset + size)  self-size) {
PyErr_SetString(PyExc_ValueError, flush values out of range);
return NULL;
}
#ifdef MS_WINDOWS
return PyInt_FromLong((long) FlushViewOfFile(self-data+offset, size));
#elif defined(UNIX)
/* XXX semantics of return value? */
/* XXX flags for msync? */
if (-1 == msync(self-data + offset, size, MS_SYNC)) {
PyErr_SetFromErrno(mmap_module_error);
return NULL;
}
return PyInt_FromLong(0);
#else
PyErr_SetString(PyExc_ValueError, flush not supported on this system);
return NULL;
#endif
}

--
components: Library (Lib)
messages: 62427
nosy: schmir
severity: major
status: open
title: mmap.flush does not check for errors on windows
type: behavior
versions: Python 2.5, Python 2.6, Python 3.0

__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2122
__
___
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com