On Wed, Nov 16, 2022 at 12:28 AM Peter Eisentraut < peter.eisentr...@enterprisedb.com> wrote:
> On 16.11.22 04:31, Ted Yu wrote: > > On Wed, 16 Nov 2022 at 11:15, Ted Yu <yuzhih...@gmail.com > > <mailto:yuzhih...@gmail.com>> wrote: > > > On Tue, Nov 15, 2022 at 7:12 PM Japin Li <japi...@hotmail.com > > <mailto:japi...@hotmail.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