[PATCH 4 of 6] bmstore: close file in a finally block in _writerepo
Augie Fackler
raf at durin42.com
Wed Dec 2 10:48:39 CST 2015
> On Dec 2, 2015, at 11:42 AM, Simon King <simon at simonking.org.uk> wrote:
>
> On Wed, Dec 2, 2015 at 4:19 PM, Augie Fackler <raf at durin42.com> wrote:
>> # HG changeset patch
>> # User Augie Fackler <augie at google.com>
>> # Date 1447293828 18000
>> # Wed Nov 11 21:03:48 2015 -0500
>> # Node ID 293c2a5543cbe1243e89592c80d24b634f6c99c7
>> # Parent 6a78f21222c1cee841367433bd354ec0402b6346
>> bmstore: close file in a finally block in _writerepo
>>
>> Also rename the variable to file_ to avoid shadowing a builtin.
>>
>> diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
>> --- a/mercurial/bookmarks.py
>> +++ b/mercurial/bookmarks.py
>> @@ -132,9 +132,11 @@ class bmstore(dict):
>> wlock = repo.wlock()
>> try:
>>
>> - file = repo.vfs('bookmarks', 'w', atomictemp=True)
>> - self._write(file)
>> - file.close()
>> + file_ = repo.vfs('bookmarks', 'w', atomictemp=True)
>> + try:
>> + self._write(file_)
>> + finally:
>> + file_.close()
>>
> If an exception occurs in self._write, won't this overwrite the bookmarks file with a potentially half-written one? Should you use something like this instead:
>
> try:
> self._write(file_)
> except:
> file_.discard()
> raise
> else:
> file_.close()
Good catch. I’ll mail a v2 presently.
>
>
> Simon
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20151202/69447bbc/attachment.pgp>
More information about the Mercurial-devel
mailing list