Re: closing file in adjust_data_dir

On Wed, Nov 16, 2022 at 12:28 AM Peter Eisentraut <
peter(dot)eisentraut(at)enterprisedb(dot)com> wrote:

> On 16.11.22 04:31, Ted Yu wrote:
> > On Wed, 16 Nov 2022 at 11:15, Ted Yu <yuzhihong(at)gmail(dot)com
> > <mailto:yuzhihong(at)gmail(dot)com>> wrote:
> > > On Tue, Nov 15, 2022 at 7:12 PM Japin Li <japinli(at)hotmail(dot)com
> > <mailto:japinli(at)hotmail(dot)com>> wrote:
> > >> After some rethinking, I find the origin code do not have
> problems.
> > >>
> > >> If fd is NULL or fgets() returns NULL, the process exits.
> > Otherwise, we
> > >> call
> > >> pclose() to close fd. The code isn't straightforward, however,
> > it is
> > >> correct.
> >
> > Hi,
> > Please take a look at the following:
> >
> > https://en.cppreference.com/w/c/io/fgets
> > <https://en.cppreference.com/w/c/io/fgets>
> > Quote: If the failure has been caused by some other error, sets the
> > /error/ indicator (see ferror()
> > <https://en.cppreference.com/w/c/io/ferror>) on |stream|. The contents
> > of the array pointed to by |str| are indeterminate (it may not even be
> > null-terminated).
>
> That has nothing to do with the return value of fgets().
>
> Hi, Peter:
Here is how the return value from pclose() is handled in other places:

+ if (pclose_rc != 0)
+ {
+ ereport(ERROR,

The above is very easy to understand.
While the check in `adjust_data_dir` is somewhat harder to comprehend.

I think the formation presented in patch v1 aligns with existing checks of
the return value from pclose().
It also gives a unique error message in the case that the return value from
pclose() indicates an error.

Cheers