On 04/23/2016 05:30 PM, Christian Ullrich wrote:
* Andrew Dunstan wrote:

On 04/22/2016 01:21 AM, Michael Paquier wrote:

5. It also complains about us casting a pid_t to a HANDLE in
pg_basebackup.c. Not sure what to do about that.
The thing that's being cast is not a PID, but a HANDLE to a process.
pid_t is a typedef for int (in port/win32.h), therefore is always 32
bits, while HANDLE is actually void*. However, Microsoft guarantees
that kernel32 HANDLEs (this includes those to threads and processes)
fit into 32 bits on AMD64.

Yes, when casting things this way I think that a comment would be fine
in the code. We could do that as separate patches actually.

We are already casting the pid_t to HANDLE and still getting a warning.
Apparently we need to do something on win64 like

    (HANDLE) ((int64) bgchild)

Ah, OK, it warns about a cast to a larger type because the value might get sign extended. Not unreasonable.

In this case, I would prefer this:

diff --git a/src/include/port/win32.h b/src/include/port/win32.h
index ba8cf9d..b4086f1 100644
--- a/src/include/port/win32.h
+++ b/src/include/port/win32.h
@@ -256,7 +256,7 @@ typedef int gid_t;
 typedef long key_t;

-typedef int pid_t;
+typedef intptr_t pid_t;


With this change, pg_basebackup -X stream works the same when built for 64 and 32 bits.

That's a change that will have a pretty wide effect. Everything up to now has been pretty low risk, but this worries me rather more. Maybe it's safe, but I'd like to hear others' comments.



Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to