Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace alias definitions with ctypes.wintypes imports for #662 #694

Merged
merged 5 commits into from
Dec 9, 2024
+7 −15
Merged

Replace alias definitions with ctypes.wintypes imports for #662 #694

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d16ef33
Replace alias definitions with ctypes.wintypes
fmtabler Dec 8, 2024
5e7c842
Fix formatting in GUID.py and git.py
fmtabler Dec 8, 2024
3005c61
Add noqa to DWORD import in __init__.py
fmtabler Dec 9, 2024
4faed94
Fix formatting in __init__.py
fmtabler Dec 9, 2024
96162f7
Fix space formatting in __init__.py
fmtabler Dec 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions comtypes/GUID.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
"""comtypes.GUID module"""

from ctypes import oledll, windll
from ctypes import byref, c_byte, c_ushort, c_ulong, c_wchar_p, Structure
from ctypes import byref, c_wchar_p, Structure
from ctypes.wintypes import BYTE, WORD, DWORD
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably don't want to use BYTE
See this issue: python/cpython#60580

Copy link
Contributor Author

@fmtabler fmtabler Dec 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to Microsoft Win32 API GUID documentation (https://learn.microsoft.com/en-us/windows/win32/api/guiddef/ns-guiddef-guid), the correct GUID structure would have unsigned chars for the Data4 field.

I think since ctypes.wintypes was updated to have BYTE = ctypes.c_ubyte here: python/cpython#97579, importing BYTE from ctypes.wintypes should be better aligned with the GUID structure than defining BYTE = c_byte in GUID.py.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, c_ubyte is the correct type.
My point is more that wintypes.DWORD = c_ubyte only since python 3.12.
So, for python 3.8 to 3.11, wintypes.DWORD = c_byte.
This may cause an unexpected behaviour because it is not the same value for all the supported python version

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@moi15moi

Is wintypes.DWORD a typo for wintypes.BYTE?
(Both DWORD in 3.11 and DWORD in 3.12 are aliases for c_ulong, and neither c_byte nor c_ubyte.)

Indeed, I overlooked python/cpython#60580. However, I believe this change is not breaking but rather a bug fix.

While it may have been possible to assign signed values to the Data4 field in the past, this was limited to the scope of ctypes structures.
Such values should not have been allowed as part of a COM component.

Tests for GUID are already included in the CI pipeline, so I don't believe there is much to fear here.

If we later discover that this change has unintended consequences after the release, we plan to revert the replacement of BYTE and release a new version.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is wintypes.DWORD a typo for wintypes.BYTE?

Yes, sorry about that.
To be sure that BYTE is c_ubyte on any python version comtypes support, shouldn't we do this?

if sys.version_info >= (3, 12):
    from ctypes.wintypes import BYTE
else:
    from ctypes import c_ubyte
    BYTE = c_ubyte

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@moi15moi

I think that’s a great idea.
Feel free to open the PR!
Let’s also check if the tests in the CI pipeline pass there.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened the issue about this: #696 (comment)

from typing import Any, TYPE_CHECKING

if TYPE_CHECKING:
Expand All @@ -12,10 +13,6 @@ def binary(obj: "GUID") -> bytes:
return bytes(obj)


BYTE = c_byte
WORD = c_ushort
DWORD = c_ulong

_ole32 = oledll.ole32

_StringFromCLSID = _ole32.StringFromCLSID
Expand Down
2 changes: 1 addition & 1 deletion comtypes/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from ctypes import HRESULT # noqa
from ctypes import _Pointer, _SimpleCData # noqa
from ctypes import c_int, c_ulong, oledll, windll
from ctypes.wintypes import DWORD # noqa
import logging
import sys
from typing import TYPE_CHECKING
Expand Down Expand Up @@ -72,7 +73,6 @@ class ReturnHRESULT(Exception):

_GUID = GUID
IID = GUID
DWORD = c_ulong

wireHWND = c_ulong

Expand Down
3 changes: 1 addition & 2 deletions comtypes/_safearray.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""SAFEARRAY api functions, data types, and constants."""

from ctypes import c_uint, c_ushort, c_void_p, POINTER, Structure, WinDLL
from ctypes.wintypes import DWORD, LONG, UINT
from ctypes.wintypes import DWORD, LONG, UINT, USHORT

from comtypes import HRESULT, GUID

Expand All @@ -19,7 +19,6 @@

VARTYPE = c_ushort
PVOID = c_void_p
USHORT = c_ushort

_oleaut32 = WinDLL("oleaut32")

Expand Down
2 changes: 1 addition & 1 deletion comtypes/errorinfo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import sys
from ctypes import *
from ctypes.wintypes import DWORD
from comtypes import IUnknown, HRESULT, COMMETHOD, GUID, BSTR
from comtypes.hresult import *

LPCOLESTR = c_wchar_p
DWORD = c_ulong


class ICreateErrorInfo(IUnknown):
Expand Down
3 changes: 1 addition & 2 deletions comtypes/git.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"""

from ctypes import *
from ctypes.wintypes import DWORD
from comtypes import (
IUnknown,
STDMETHOD,
Expand All @@ -15,8 +16,6 @@
CLSCTX_INPROC_SERVER,
)

DWORD = c_ulong


class IGlobalInterfaceTable(IUnknown):
_iid_ = GUID("{00000146-0000-0000-C000-000000000046}")
Expand Down
5 changes: 1 addition & 4 deletions comtypes/typeinfo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

import ctypes
from ctypes import HRESULT, POINTER, _Pointer, byref, c_int, c_void_p, c_wchar_p
from ctypes.wintypes import DWORD, LONG, UINT, ULONG, WCHAR, WORD
from ctypes.wintypes import DWORD, LONG, UINT, ULONG, WCHAR, WORD, INT, SHORT, USHORT
from comtypes import BSTR, _CData, COMMETHOD, GUID, IID, IUnknown, STDMETHOD
from comtypes.automation import DISPID, LCID, SCODE
from comtypes.automation import DISPPARAMS, EXCEPINFO, VARIANT, VARIANTARG, VARTYPE
Expand All @@ -28,15 +28,12 @@

BOOL = c_int
HREFTYPE = DWORD
INT = c_int
MEMBERID = DISPID
OLECHAR = WCHAR
PVOID = c_void_p
SHORT = ctypes.c_short
# See https://msdn.microsoft.com/en-us/library/windows/desktop/aa383751(v=vs.85).aspx#ULONG_PTR # noqa
ULONG_PTR = ctypes.c_uint64 if is_64_bit else ctypes.c_ulong

USHORT = ctypes.c_ushort
LPOLESTR = POINTER(OLECHAR)

################################################################
Expand Down