classification
Title: (minor) mismatched argument in overlapped_RegisterWaitWithQueue call to RegisterWaitForSingleObject
Type: Stage: resolved
Components: IO Versions: Python 3.9, Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Alexander Riccio, ZackerySpytz, miss-islington, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2020-04-02 03:13 by Alexander Riccio, last changed 2020-07-15 19:53 by serhiy.storchaka. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 19686 merged ZackerySpytz, 2020-04-23 21:02
PR 21493 merged miss-islington, 2020-07-15 18:43
PR 21494 merged miss-islington, 2020-07-15 18:43
Messages (4)
msg365565 - (view) Author: Alexander Riccio (Alexander Riccio) * Date: 2020-04-02 03:13
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.
msg373708 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-07-15 18:43
New changeset af4eda46d1538b1da700a86588bdb94b0a4d1ff2 by Zackery Spytz in branch 'master':
bpo-40150: Fix mismatched argument in RegisterWaitForSingleObject() call (GH-19686)
https://github.com/python/cpython/commit/af4eda46d1538b1da700a86588bdb94b0a4d1ff2
msg373712 - (view) Author: miss-islington (miss-islington) Date: 2020-07-15 19:25
New changeset f8055fb0f1054fce6a21047da39b92ae32416b80 by Miss Islington (bot) in branch '3.8':
bpo-40150: Fix mismatched argument in RegisterWaitForSingleObject() call (GH-19686)
https://github.com/python/cpython/commit/f8055fb0f1054fce6a21047da39b92ae32416b80
msg373713 - (view) Author: miss-islington (miss-islington) Date: 2020-07-15 19:26
New changeset 4a02da4f95cfff679e38a13ca0f44d660c5669b5 by Miss Islington (bot) in branch '3.9':
bpo-40150: Fix mismatched argument in RegisterWaitForSingleObject() call (GH-19686)
https://github.com/python/cpython/commit/4a02da4f95cfff679e38a13ca0f44d660c5669b5
History
Date User Action Args
2020-07-15 19:53:03serhiy.storchakasetstatus: open -> closed
stage: patch review -> resolved
resolution: fixed
versions: + Python 3.7, Python 3.8
2020-07-15 19:26:06miss-islingtonsetmessages: + msg373713
2020-07-15 19:25:59miss-islingtonsetmessages: + msg373712
2020-07-15 18:43:27miss-islingtonsetpull_requests: + pull_request20636
2020-07-15 18:43:19miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request20635
2020-07-15 18:43:07serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg373708
2020-04-23 21:02:35ZackerySpytzsetkeywords: + patch
nosy: + ZackerySpytz

pull_requests: + pull_request19005
stage: patch review
2020-04-02 03:13:59Alexander Ricciocreate