Re: [PATCHv1 4/6] git-p4: create helper function importRevisions()
- From
Luke Diamand <luke@diamand.org>
- Date
- Jan 31, 2020, 10:36 UTC
- Message-ID
- <CAE5ih78VwT3iRb8pmyN9g-JnU2dET2ZZ4X8CHWSxdARtq3-tEg@mail.gmail.com>
- In-Reply-To
- <xmqqzhe4btr0.fsf@gitster-ct.c.googlers.com>
On Thu, 30 Jan 2020 at 19:59, Junio C Hamano <gitster@pobox.com> wrote:
Show 30 quoted lines
> > Eric Sunshine <sunshine@sunshineco.com> writes: > > > On Wed, Jan 29, 2020 at 6:13 AM Luke Diamand <luke@diamand.org> 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@diamand.org> > >> --- > >> 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.