| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
|---|---|
| To: | Bryan Green <dbryan(dot)green(at)gmail(dot)com> |
| Cc: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain |
| Date: | 2025-12-13 06:33:15 |
| Message-ID: | 1115993.1765607595@sss.pgh.pa.us |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Bryan Green <dbryan(dot)green(at)gmail(dot)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.
> 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];
> 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).
BTW, pgindent will undo some of the whitespace changes you made
here.
regards, tom lane
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Roman Khapov | 2025-12-13 07:44:46 | Additional message in pg_terminate_backend |
| Previous Message | jian he | 2025-12-13 05:16:28 | virtual generated column as partition key |