Re: closing file in adjust_data_dir

From: Ted Yu <yuzhihong(at)gmail(dot)com>
To: Japin Li <japinli(at)hotmail(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: closing file in adjust_data_dir
Date: 2022-11-16 02:06:34
Message-ID: CALte62ziz4DgwLBwaBwPZT11BwCz-io2qpnYz2n6DFMDXUEOZA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Nov 15, 2022 at 6:02 PM Japin Li <japinli(at)hotmail(dot)com> wrote:

>
> On Wed, 16 Nov 2022 at 02:43, Ted Yu <yuzhihong(at)gmail(dot)com> wrote:
> > Hi,
> > I was looking at the commit:
> >
> > commit 2fe3bdbd691a5d11626308e7d660440be6c210c8
> > Author: Peter Eisentraut <peter(at)eisentraut(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Japin Li 2022-11-16 02:09:00 Re: closing file in adjust_data_dir
Previous Message Amit Kapila 2022-11-16 02:03:42 Re: hash_xlog_split_allocate_page: failed to acquire cleanup lock