On Thu, Jun 14, 2012 at 1:48 PM, Gurjeet Singh <singh.gurjeet@gmail.com> wrote:
> On Thu, Jun 14, 2012 at 1:29 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>>
>> On Thu, Jun 14, 2012 at 6:06 AM, Gabriele Bartolini
>> <gabriele.bartolini@2ndquadrant.it> wrote:
>> > thank you very much for your patience (and thank you Marco for
>> > supporting
>> > me). I apologise for the delay.
>> >
>> > I have retested the updated patch and it works fine with me. It is
>> > "ready
>> > for committer" for me.
>>
>> Committed.
>
>
> A minor gripe:
>
> + /*
> + * Close the backup label file.
> + */
> + if (ferror(lfp) || FreeFile(lfp)) {
> + ereport(ERROR,
> + (errcode_for_file_access(),
> + errmsg("could not read file \"%s\": %m",
> + BACKUP_LABEL_FILE)));
> + }
> +
>
> If ferror(lfp) returns false, wouldn't we miss the FreeFile() and leak a
> file pointer?
Well, according to the comments for AllocateFile:
* fd.c will automatically close all files opened with AllocateFile at* transaction commit or abort; this prevents FD
leakageif a routine* that calls AllocateFile is terminated prematurely by ereport(ERROR).
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company