[PATCH 1 of 4] hgmerge: add new hgmerge package under mercurial

Matt Mackall mpm at selenic.com
Tue Jan 8 16:25:20 CST 2008


On Tue, 2008-01-08 at 22:35 +0100, Arve Knudsen wrote:
>         > > +def _readfile(fname):
>         > > +    f = file(fname, 'rb') 
>         > > +    try: data = f.read()
>         > > +    finally: f.close()
>         > > +    return data
>         
>         >         This function should be taken out and shot.
>         
>         > I don't see why, but what do I know. 
>         
>         
>         What's it accomplishing? The only thing it does is close a
>         file slightly
>         earlier than it would otherwise be closed in the case of an
>         exception.
>         If that matters, we're probably hiding a real bug somewhere
>         else. 
>         
>         
> 
> I always prefer to close files right away, in which case this simple
> function makes sense. The reason I want to close files right away is
> to avoid possible conflicts due to open files on Windows ( e.g.,
> trying to deleting an open file). From testing right now, on Windows
> XP, it appears that an orphaned file object is closed implicitly
> before the subsequent call to os.remove, though. I don't know if this
> is down to pure luck (the gc getting there before os.remove), but I
> prefer being conservative.

f gets deleted at the return unless an exception occurs. In that case,
the stack frame for the function exists until the exception object is
destroyed. If we hold onto an exception object long enough for this to
be an issue, we have bigger problems.

-- 
Mathematics is the supreme nostalgia of our time.



More information about the Mercurial-devel mailing list