On Thu, 30 Jan 2020 at 19:59, Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes: > > > On Wed, Jan 29, 2020 at 6:13 AM Luke Diamand <luke@xxxxxxxxxxx> wrote: > >> This makes it easier to try/catch around this block of code to ensure > >> cleanup following p4 failures is handled properly. > >> > >> Signed-off-by: Luke Diamand <luke@xxxxxxxxxxx> > >> --- > >> diff --git a/git-p4.py b/git-p4.py > >> @@ -3555,6 +3555,73 @@ def importHeadRevision(self, revision): > >> + def importRevisions(self, args, branch_arg_given): > >> + if len(self.changesFile) > 0: > >> + output = open(self.changesFile).readlines() > > > > Not a new problem (since this code is merely being relocated), but is > > this leaking the open file? Should there be an accompanying close() > > somewhere? > > > > f = open(self.changesFile) > > output = f.readlines() > > close(f) > > > > or something. > > Hmph, I was naively hoping that the (never assigned to any variable) > last reference going away at the end of the statement would make the > file object dead, and we can let eventual GC to close it. > > Nevertheless it would not hurt to explicitly control the lifetime. You are right, there is no file descriptor leak. It's easy enough to write some test code to demonstrate this.
