win: Add HandleToInt() and IntToHandle()

win: Add HandleToInt() and IntToHandle()

This consolidates all of the twisted casts and comments that discuss how
HANDLEs are really only 32 bits wide even in 64-bit processes on 64-bit
operating systems into a single location.

R=scottmg@chromium.org

Committed: https://chromium.googlesource.com/crashpad/crashpad/+/2eeaa3ac549a6646031cac05f8d1424ff4946157

[Mark Mentovai, 2015-11-05 17:35:37]

mark@chromium.org changed reviewers:
+ scottmg@chromium.org

[scottmg, 2015-11-05 18:03:18]

Nice, good idea.

https://codereview.chromium.org/1422503015/diff/1/util/win/handle.cc
File util/win/handle.cc (right):

https://codereview.chromium.org/1422503015/diff/1/util/win/handle.cc#newcode24
util/win/handle.cc:24: return
base::checked_cast<int>(reinterpret_cast<intptr_t>(handle));
I would have made these uintptr_t and uint32_t. Do handles have to be be <
0x8000`0000? Otherwise it seems odd to have them go negative especially when
they're being sent as %xs.

https://codereview.chromium.org/1422503015/diff/1/util/win/process_info.h
File util/win/process_info.h (right):

https://codereview.chromium.org/1422503015/diff/1/util/win/process_info.h#new...
util/win/process_info.h:61: uint32_t handle;
Similarly here since the conversion is HANDLE -> int -> uint32_t.

[Mark Mentovai, 2015-11-05 18:26:14]

https://codereview.chromium.org/1422503015/diff/1/util/win/handle.cc
File util/win/handle.cc (right):

https://codereview.chromium.org/1422503015/diff/1/util/win/handle.cc#newcode24
util/win/handle.cc:24: return
base::checked_cast<int>(reinterpret_cast<intptr_t>(handle));
scottmg wrote:
> I would have made these uintptr_t and uint32_t. Do handles have to be be <
> 0x8000`0000? Otherwise it seems odd to have them go negative especially when
> they're being sent as %xs.

INVALID_HANDLE_VALUE is -1, so we have to use ints for sign-extension to do the
right thing, otherwise in 64-bit processes, we’d convert (HANDLE)
INVALID_HANDLE_VALUE → (unsigned) 0xffffffff → (HANDLE) 0xffffffff, which isn’t
a nice clean round-trip.

I could do static_cast<unsigned
int>(base::checked_cast<int>(reinterpret_cast<intptr_t>(handle))); (or uint32_t
instead of unsigned int) if that would be better.

I could also just document why I chose int. Probably makes more sense in this
.cc than in the .h, since it’s kind of a detail.

What do you think?

[scottmg, 2015-11-05 18:40:04]

lgtm

https://codereview.chromium.org/1422503015/diff/1/util/win/handle.cc
File util/win/handle.cc (right):

https://codereview.chromium.org/1422503015/diff/1/util/win/handle.cc#newcode24
util/win/handle.cc:24: return
base::checked_cast<int>(reinterpret_cast<intptr_t>(handle));
On 2015/11/05 18:26:14, Mark Mentovai wrote:
> scottmg wrote:
> > I would have made these uintptr_t and uint32_t. Do handles have to be be <
> > 0x8000`0000? Otherwise it seems odd to have them go negative especially when
> > they're being sent as %xs.
> 
> INVALID_HANDLE_VALUE is -1, so we have to use ints for sign-extension to do
the
> right thing, otherwise in 64-bit processes, we’d convert (HANDLE)
> INVALID_HANDLE_VALUE → (unsigned) 0xffffffff → (HANDLE) 0xffffffff, which
isn’t
> a nice clean round-trip.
> 
> I could do static_cast<unsigned
> int>(base::checked_cast<int>(reinterpret_cast<intptr_t>(handle))); (or
uint32_t
> instead of unsigned int) if that would be better.
> 
> I could also just document why I chose int. Probably makes more sense in this
> .cc than in the .h, since it’s kind of a detail.
> 
> What do you think?

Gotcha. I think what you did seem right then. We should probably changed the
uint32_ts in the registration protocol and elsewhere to ints in that case.

[Mark Mentovai, 2015-11-05 19:00:33]

Committed patchset #2 (id:20001) manually as
2eeaa3ac549a6646031cac05f8d1424ff4946157 (presubmit successful).