On Tue, Sep 05, 2023 at 02:06:14PM +0200, Daniel Gustafsson wrote:
>> For that purpose it's actually more secure to use all bits for random
>> data, instead of keeping one bit always 0.
>
> If it's common practice to submit a pid which isn't a pid, I wonder if longer
> term it's worth inventing a value for be_pid which means "unknown pid" such
> that consumers can make informed calls when reading it? Not the job of this
> patch to do so, but maybe food for thought.
Perhaps.
>> So, while I agree that putting a negative value in the process ID field of
>> BackendData, is arguably incorrect. Given the simplicity of the fix on
>> the pg_basebackup side, I think addressing it in pg_basebackup is
>> easier than fixing this in all connection poolers.
>
> Since the value in the temporary slotname isn't used to convey meaning, but
> merely to ensure uniqueness, I don't think it's unreasonable to guard aginst
> malformed input (ie negative integer).
PQbackendPID() returns a signed value, likely coming from the fact
that it was thought to be OK back in the days where PIDs were always
defined with less bits. The fix is OK taken in isolation, so I am
going to apply it in a few minutes as I'm just passing by..
Saying that, I agree with the point that we should also use %u in
psql's prompt.c to get a correct PID if the 32th bit is set, and move
on.
--
Michael