Re: pg_ctl start may return 0 even if the postmaster has been already started on Windows

At Fri, 8 Sep 2023 08:02:57 +0000, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com> wrote in 
> > > Ditching cmd.exe seems like a big hassle. So, on the flip side, I
> > > tried to identify the postmaster PID using the shell's PID, and it
> > > seem to work. The APIs used are avaiable from XP/2003 onwards.
> 
> According to 495ed0ef2, Windows 10 seems the minimal requirement for using
> the postgres. So the approach seems OK.
> 
> Followings are my comment, but I can say only cosmetic ones because I do not have
> windows machine which can run postgres.

Thank you for the comment!

> 1.
> Forward declaration seems missing. In the pg_ctl.c, the static function seems to
> be declared even if there is only one caller (c.f., GetPrivilegesToDelete).

Agreed. 

> 2.
> I think the argument should be pid_t.

Yeah, I didn't linger on that detail earlier. But revisiting it, I
coucur it is best suited since it is a local function in
pg_ctl.c. I've now positioned it at the end of a WIN32 section
defining other win32-specific functions. Hence, a forward declaration
became necessary:p

> 3.
> I'm not sure the return type of the function should be pid_t or not. According
> to the document, DWORD corrresponds to the pid_t. In win32_port.h, the pid_t is
> defiend as int (_MSC_VER seems to be defined when the VisualStduio is used). It
> is harmless, but I perfer to match the interface between caller/callee. IIUC we
> can add just a cast.

For the reason previously stated, I've adjusted the type for both the
parameter and the return value to pid_t. start_postmaster() already
assumed that pid_t is wider than DWORD.

I noticed that PID 0 is valid on Windows. However, it is consistently
the PID for the system idle process, so it can't be associated with
cmd.exe or postgres. I've added a comment noting that premise. Also I
did away with an unused variable.  For the CreateToolhelp32Snapshot
function, I changed the second parameter to 0 from shell_pid, since it
is not used when using TH32CS_SNAPPROCESS.  I changed the comparison
operator for pid_t from > to !=, ensuring correct behavior even with
negative values.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center