Re: Patch pg_is_in_backup()

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:
>> 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 leakage if a routine
 * that calls AllocateFile is terminated prematurely by ereport(ERROR).