On Tue, Nov 15, 2022 at 6:02 PM Japin Li <japi...@hotmail.com> wrote:
> > On Wed, 16 Nov 2022 at 02:43, Ted Yu <yuzhih...@gmail.com> wrote: > > Hi, > > I was looking at the commit: > > > > commit 2fe3bdbd691a5d11626308e7d660440be6c210c8 > > Author: Peter Eisentraut <pe...@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