classification
Title: dupe self.fp.tell() in zipfile.ZipFile.writestr
Type: performance Stage: resolved
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ezio.melotti Nosy List: alanmcintyre, ezio.melotti, proppy, python-dev, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2011-05-02 16:36 by proppy, last changed 2012-11-17 16:57 by ezio.melotti. This issue is now closed.

Files
File name Uploaded Description Edit
zipfile-fix-dupe-fp-tell.patch proppy, 2011-05-03 13:39 patch review
zinfo.header_offset.log proppy, 2011-05-05 12:41
Messages (9)
msg134990 - (view) Author: Johan Euphrosine (proppy) Date: 2011-05-02 16:36
See:
http://hg.python.org/cpython/file/2e3346fc880f/Lib/zipfile.py#l1168
http://hg.python.org/cpython/file/2e3346fc880f/Lib/zipfile.py#l1182
msg135188 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2011-05-05 11:36
Can you prove that the second one is useless (did it cause you any problem or did you just find it reading the source)?
There might be reasons why there are two calls to fp.tell() (e.g. the result is different in the two places and/or zinfo.header_offset is read/changed somewhere else between the two calls).
msg135192 - (view) Author: Johan Euphrosine (proppy) Date: 2011-05-05 12:11
I just find it while reading the source, for fixing #11980

zinfo.header_offset is only read in self._write_check, and it seems to me that no file operation are performed on self.fp between the two call. So I can't see see how it could different.

Also the tests suite still pass after removing the second one :)

Let me know if you want me to do more investigation.
msg135194 - (view) Author: Johan Euphrosine (proppy) Date: 2011-05-05 12:41
Here is a log that shows zinfo.header_offset value after each .tell() when running test_zipfile
msg135960 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2011-05-14 06:56
I double checked the code on py3k and I think the second occurrence can be removed.
msg143970 - (view) Author: Alan McIntyre (alanmcintyre) * (Python committer) Date: 2011-09-13 14:06
I also can't see any file operations that might occur between the two .tell() calls, and a full test pass (including test_zipfile64) on the py3k development branch doesn't turn up any problems on Linux (2.6.38, x86_64) for me, so I agree the second .tell() could be removed.
msg175398 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-11-11 21:56
LGTM.
msg175772 - (view) Author: Roundup Robot (python-dev) Date: 2012-11-17 16:56
New changeset d51665f9a416 by Ezio Melotti in branch 'default':
#11981: remove duplicate line.  Patch by Johan Euphrosine.
http://hg.python.org/cpython/rev/d51665f9a416
msg175773 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2012-11-17 16:57
Fixed, thanks for the patch!
History
Date User Action Args
2012-11-17 16:57:00ezio.melottisetstatus: open -> closed
versions: - Python 3.3
messages: + msg175773

assignee: ezio.melotti
resolution: fixed
stage: commit review -> resolved
2012-11-17 16:56:26python-devsetnosy: + python-dev
messages: + msg175772
2012-11-11 21:56:21serhiy.storchakasetstage: commit review
messages: + msg175398
versions: + Python 3.4
2012-04-07 17:57:55serhiy.storchakasetnosy: + serhiy.storchaka
2011-09-13 14:06:45alanmcintyresetmessages: + msg143970
2011-05-14 06:56:29ezio.melottisetnosy: + alanmcintyre

messages: + msg135960
versions: + Python 3.3, - Python 3.4
2011-05-05 12:41:53proppysetfiles: + zinfo.header_offset.log

messages: + msg135194
2011-05-05 12:11:43proppysetmessages: + msg135192
2011-05-05 11:36:41ezio.melottisetnosy: + ezio.melotti
messages: + msg135188
2011-05-03 13:39:06proppysetfiles: + zipfile-fix-dupe-fp-tell.patch
keywords: + patch
title: duplicated self.fp.tell() in zipfile.ZipFile.writestr -> dupe self.fp.tell() in zipfile.ZipFile.writestr
2011-05-02 16:37:17proppysettype: performance
components: + Library (Lib)
2011-05-02 16:36:57proppycreate