Re: ParseTzFile doesn't FreeFile on error

From: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
To: tgl(at)sss(dot)pgh(dot)pa(dot)us
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: ParseTzFile doesn't FreeFile on error
Date: 2022-05-31 00:22:37
Message-ID: 20220531.092237.2228622691835648951.horikyota.ntt@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Mon, 30 May 2022 13:11:04 -0400, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote in
> Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> writes:
> > The cause is ParseTzFile() returns leaving an open file descriptor
> > unfreed in some error cases.
> > This happens only in a special case when the errors are ignored, but
> > in principle the file descriptor should be released before exiting the
> > function.
> > I'm not sure it's worth fixing but the attached fixes that.
>
> I agree this is worth fixing, but adding all these gotos seems a bit
> inelegant. What do you think of the attached version?

It is what came up to me first. It is natural. So I'm fine with
it. The point of the "goto"s was that repeated "n = -1;break;" looked
somewhat noisy to me in the loop.

> BTW, my first thought about it was "what if one of the callees throws
> elog(ERROR), eg palloc out-of-memory"? But I think that's all right
> since then we'll reach transaction abort cleanup, which won't whine
> about open files. The problem is limited to the case where no error
> gets thrown.

Right. This "issue" is not a problem unless the caller continues
without throwing an exception after the function errors out, which is
not done by the current code.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2022-05-31 00:31:12 Re: "ERROR: latch already owned" on gharial
Previous Message Thomas Munro 2022-05-31 00:08:23 Re: "ERROR: latch already owned" on gharial