(minor) mismatched argument in overlapped_RegisterWaitWithQueue call to RegisterWaitForSingleObject

[ariccio]

BPO	40150
Nosy	@serhiy-storchaka, @ariccio, @ZackerySpytz, @miss-islington
PRs	bpo-40150: Fix mismatched argument in RegisterWaitForSingleObject() call #19686 [3.9] bpo-40150: Fix mismatched argument in RegisterWaitForSingleObject() call (GH-19686) #21493 [3.8] bpo-40150: Fix mismatched argument in RegisterWaitForSingleObject() call (GH-19686) #21494

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2020-07-15.19:53:03.988>
created_at = <Date 2020-04-02.03:13:59.780>
labels = ['3.7', '3.8', '3.9', 'expert-IO']
title = '(minor) mismatched argument in overlapped_RegisterWaitWithQueue call to RegisterWaitForSingleObject'
updated_at = <Date 2020-07-15.19:53:03.988>
user = 'https://github.com/ariccio'

bugs.python.org fields:

activity = <Date 2020-07-15.19:53:03.988>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2020-07-15.19:53:03.988>
closer = 'serhiy.storchaka'
components = ['IO']
creation = <Date 2020-04-02.03:13:59.780>
creator = 'Alexander Riccio'
dependencies = []
files = []
hgrepos = []
issue_num = 40150
keywords = ['patch']
message_count = 4.0
messages = ['365565', '373708', '373712', '373713']
nosy_count = 4.0
nosy_names = ['serhiy.storchaka', 'Alexander Riccio', 'ZackerySpytz', 'miss-islington']
pr_nums = ['19686', '21493', '21494']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue40150'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

[ariccio]

This popped out at me while looking for something else. It's probably not much of an actual problem, since the wrong datatype is larger than the correct one, but it's worth fixing.

The problem is in overlapped_RegisterWaitWithQueue, at overlapped.c:297 (in _overlapped):

if (!RegisterWaitForSingleObject(
        &NewWaitObject, Object, (WAITORTIMERCALLBACK)PostToQueueCallback,
        pdata, Milliseconds,
        WT_EXECUTEINWAITTHREAD | WT_EXECUTEONLYONCE))

...it stood out to me immediately while I was paging past, since function pointer casts of this sort are almost always wrong, and I've seen it too many times.

WAITORTIMERCALLBACK is a typedef of WAITORTIMERCALLBACKFUNC:

typedef VOID (NTAPI * WAITORTIMERCALLBACKFUNC) (PVOID, BOOLEAN );

...and PostToQueueCallback is defined as:

static VOID CALLBACK
PostToQueueCallback(PVOID lpParameter, BOOL TimerOrWaitFired)

...where BOOL is an int, and BOOLEAN is an unsigned char. I guess there could be some kind of issue down the line if the generated code tries to read the BOOLEAN/int from the register/memory location where there's actually an unsigned char, but after about an hour of debugging, I can't see it. The documentation also states explicitly that this should be either TRUE or FALSE. Also, it doesn't matter what we actually pass to PostQueuedCompletionStatus: https://devblogs.microsoft.com/oldnewthing/20070525-00/?p=26693

By changing the (incorrect) BOOL declaration to BOOLEAN, then we don't need the cast.

[serhiy-storchaka]

New changeset af4eda4 by Zackery Spytz in branch 'master':
bpo-40150: Fix mismatched argument in RegisterWaitForSingleObject() call (GH-19686)
af4eda4

[miss-islington]

New changeset f8055fb by Miss Islington (bot) in branch '3.8':
bpo-40150: Fix mismatched argument in RegisterWaitForSingleObject() call (GH-19686)
f8055fb

[miss-islington]

New changeset 4a02da4 by Miss Islington (bot) in branch '3.9':
bpo-40150: Fix mismatched argument in RegisterWaitForSingleObject() call (GH-19686)
4a02da4