Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain

On Sat, Dec 13, 2025 at 7:33 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Bryan Green <dbryan.green@gmail.com> writes:
> > I removed the useless snprintf() call that was using GetCommandLine().
> > That was left in place when I moved to GetModuleFileName().  Also,
> > removed the unused 'space_pos' variable and the unneeded scope block.
>
> All good to my eye.

Thanks both.

> > I decided to just use 1024 for the exe_path size since that is what
> > cmdline is set to use.
>
> Personally I'd have gone the other way, say
>
>         char            exe_path[MAXPGPATH];
>         char            cmdline[MAXPGPATH + 100];

Done in this version.

> > I also removed some self-evident comments that
> > were leftover from my practice of writing comments and then coding.
>
> I think you went way overboard on removing "self-evident" comments.
> Signposts as to what the code intends to do are pretty helpful IMO.
> They do have to be accurate though, for instance this previous
> comment:
>
> -        * Find the actual executable path by removing any arguments from
> -        * GetCommandLine().
>
> didn't seem to convey what the code was doing (which I neglected
> to complain about before).

Comments restored in the attached version of Bryan's patch.

My earlier guess about the Makefile was wrong, and when I looked into
it the actual problems were (1) that the CompilerWarnings task in CI
runs make world-bin, which doesn't descend into src/test, and (2) that
the test ifeq ($(PORTNAME), win32) was not satisfied due to make's
rules for variable evaluation.  I thought about how to fix that but
realised that this is going to be much easier to maintain if it's not
different on Unix, so here are some fixes in that direction.  With
just 0001 and 0002 applied, we'd have known about the compiler warning
before commit, with a failure like this:

https://cirrus-ci.com/task/5863371716689920

With 0003 applied on top, it's green and there are no warnings from
either Windows task:

https://cirrus-ci.com/build/4775547869331456

I also changed the comment style of some single-line comments.
replaced the memset() with initializer syntax and ran pgindent which
undid a change or two.