Re: closing file in adjust_data_dir - Mailing list pgsql-hackers

From Ted Yu
Subject Re: closing file in adjust_data_dir
Date
Msg-id CALte62wtdry5u=_Pf5AyVf4zg1hDx_6nPEmjJSg9N3oZRpD4Ug@mail.gmail.com
Whole thread Raw
In response to Re: closing file in adjust_data_dir  (Japin Li <japinli@hotmail.com>)
Responses Re: closing file in adjust_data_dir
List pgsql-hackers


On Tue, Nov 15, 2022 at 7:12 PM Japin Li <japinli@hotmail.com> wrote:

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. 

pgsql-hackers by date:

Previous
From: Japin Li
Date:
Subject: Re: closing file in adjust_data_dir
Next
From: Justin Pryzby
Date:
Subject: Re: libpq compression (part 2)