[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