nvda
91e60c70 - Isolate winBindings ctypes definitions (#18978)

Commit
248 days ago
Isolate winBindings ctypes definitions (#18978) ### Link to issue number: <!-- Use Closes/Fixes/Resolves #xxx to link this PR to the issue it is responding to. --> Fixes #18967 Fixes #18966 Fixes #18950 ### Summary of the issue: With the move to 64 bit, we have replaced all raw ctypes calls to Windows API functions with calls to well defined ctypes definitions in our new winBindings package. However, these definitions are still just references to the global ctypes functions looked up as attributes on the dlls. And then their argtypes and restye have been changed. Although these definitions are now correct, these changes have been propagated globally, meaning that these changes may break old 3rd party code that still make raw ctypes calls to these functions. And also, our new code could break as if a Python dependency sets argtypes or restype on these globally, different to our way. We need to isolate our winBindings ctypes definitions so that they do not affect, or are not affected by, 3rd party usage. ### Description of user facing changes: ### Description of developer facing changes: ### Description of development approach: In each winBindings module, locate and change all lines of the form: ``` xx = dll.yy ``` Into ``` xx = WINFUNCTYPE(None)(('yy', dll)) ``` By using the following sed command: ``` sed -E -i "s/^([[:alnum:]_]+)[[:space:]]*=[[:space:]]*dll\.([[:alnum:]_]+)[[:space:]]*$/\1 = WINFUNCTYPE(None)(('\2', dll))/" -- *.py ``` Now rather than xx being a reference to the global function pointer for that function, it is now its own isolated WINFUNCTYPE function prototype, which returns void and takes no arguments. Then, the existing code to change argtypes and restype applies, correcting this definition to match Microsoft documentation. But only affecting our isolated copy, not globally. For the majority of modules, WINFUNCTYPE also needed to be imported. Finally, several typos were identified in winBindings.user32 where argtypes was spelled as argTypes. these have been corrected. ### Testing strategy: * General smoke tests - running NVDA for a period of time. * Confirmed with timeit that there is no performance difference between calling a global function pointer and a WINFUNCTYPE function prototype. ### Known issues with pull request: It may seem a bit strange to first create the prototype returning void with no arguments, and then setting argtypes and restype separately. However, this was the quickest and least risky change we could make to introduce isolation to fix the related issues. We may consider writing an automation script to restructure the definitions so that the result and arguments are set within the WINFUNCTYPE definition. But this would be purely a style thing and has no impact on performance or runtime execution. ### Code Review Checklist: <!-- This checklist is a reminder of things commonly forgotten in a new PR. Authors, please do a self-review of this pull-request. Check items to confirm you have thought about the relevance of the item. Where items are missing (eg unit / system tests), please explain in the PR. To check an item `- [ ]` becomes `- [x]`, note spacing. You can also check the checkboxes after the PR is created. A detailed explanation of this checklist is available here: https://github.com/nvaccess/nvda/blob/master/projectDocs/dev/githubPullRequestTemplateExplanationAndExamples.md#code-review-checklist --> - [x] Documentation: - Change log entry - User Documentation - Developer / Technical Documentation - Context sensitive help for GUI changes - [x] Testing: - Unit tests - System (end to end) tests - Manual testing - [x] UX of all users considered: - Speech - Braille - Low Vision - Different web browsers - Localization in other languages / culture than English - [x] API is compatible with existing add-ons. - [x] Security precautions taken. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Sean Budd <sean@nvaccess.org>
Parents
Loading