On Wed, 16 Nov 2022 at 10:52, Ted Yu <yuzhihong@gmail.com> wrote: > On Tue, Nov 15, 2022 at 6:35 PM Japin Li <japinli@hotmail.com> wrote: >> >> fd = popen(cmd, "r"); >> - if (fd == NULL || fgets(filename, sizeof(filename), fd) == NULL || >> pclose(fd) != 0) >> + if (fd == NULL || fgets(filename, sizeof(filename), fd) == NULL) >> { >> + pclose(fd); >> write_stderr(_("%s: could not determine the data directory >> using command \"%s\"\n"), progname, cmd); >> exit(1); >> } >> >> Here, segfault maybe occurs if fd is NULL. I think we can remove pclose() >> safely since the process will exit. >> > > That means we're going back to v1 of the patch. >
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.
Please read this sentence from my first post:
If the fgets() call doesn't return NULL, the pclose() would be skipped.