Re: closing file in adjust_data_dir - Mailing list pgsql-hackers

From Ted Yu
Subject Re: closing file in adjust_data_dir
Date
Msg-id CALte62ziz4DgwLBwaBwPZT11BwCz-io2qpnYz2n6DFMDXUEOZA@mail.gmail.com
Whole thread Raw
In response to Re: closing file in adjust_data_dir  (Japin Li <japinli@hotmail.com>)
Responses Re: closing file in adjust_data_dir
List pgsql-hackers


On Tue, Nov 15, 2022 at 6:02 PM Japin Li <japinli@hotmail.com> wrote:

On Wed, 16 Nov 2022 at 02:43, Ted Yu <yuzhihong@gmail.com> wrote:
> Hi,
> I was looking at the commit:
>
> commit 2fe3bdbd691a5d11626308e7d660440be6c210c8
> Author: Peter Eisentraut <peter@eisentraut.org>
> Date:   Tue Nov 15 15:35:37 2022 +0100
>
>     Check return value of pclose() correctly
>
> In src/bin/pg_ctl/pg_ctl.c :
>
>     if (fd == NULL || fgets(filename, sizeof(filename), fd) == NULL ||
> pclose(fd) != 0)
>
> If the fgets() call doesn't return NULL, the pclose() would be skipped.
> Since the original pclose() call was removed, wouldn't this lead to fd
> leaking ?
>
> Please see attached patch for my proposal.
>
> Cheers

I think we should check whether fd is NULL or not, otherwise, segmentation
fault maybe occur.

+       if (pclose(fd) != 0)
+       {
+               write_stderr(_("%s: could not close the file following command \"%s\"\n"), progname, cmd);
+               exit(1);
+       }

Hi,
That check is a few line above:

+       if (fd == NULL || fgets(filename, sizeof(filename), fd) == NULL)
        {

Cheers 

pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: hash_xlog_split_allocate_page: failed to acquire cleanup lock
Next
From: Japin Li
Date:
Subject: Re: closing file in adjust_data_dir