Feat: Rewritten Windows Bindings - #5
Open
Mahasvan wants to merge 67 commits into
Open
Conversation
…win-bindings-new
kernel-dev
reviewed
Aug 1, 2026
Comment on lines
+21
to
+23
| typedef struct { | ||
| char values[WMI_MAX_FIELDS][WMI_FIELD_LEN]; | ||
| } WmiRow; |
Collaborator
There was a problem hiding this comment.
This could be turned into a typedef without being encapsulated in a struct. Is there a specific reason for it to stay inside of a struct?
Also, I think you meant to do char values[WMI_FIELD_LEN][WMI_MAX_FIELDS]
Collaborator
There was a problem hiding this comment.
@Mahasvan please fix this as well: you meant to do values[WMI_FIELD_LEN][WMI_MAX_FIELDS]
Comment on lines
+79
to
+88
| // VT_BSTR fast path. | ||
| if (bstr) { | ||
| int len = WideCharToMultiByte(CP_UTF8, 0, bstr, -1, nullptr, 0, nullptr, nullptr); | ||
| if (len > 0) { | ||
| if (len > dst_size) len = dst_size; | ||
| WideCharToMultiByte(CP_UTF8, 0, bstr, -1, dst, len, nullptr, nullptr); | ||
| dst[dst_size - 1] = '\0'; | ||
| } | ||
| } | ||
| } |
Collaborator
There was a problem hiding this comment.
Wouldn't it just be easier to move this into the initial if condition block?
Owner
Author
There was a problem hiding this comment.
apparently not, according to claude
Collaborator
There was a problem hiding this comment.
Why not? It seems to be doing the exact same thing when analyzing control flow?
kernel-dev
reviewed
Aug 1, 2026
…win-bindings-new
Episode 2 - Attack of the Clones
kernel-dev
approved these changes
Aug 9, 2026
kernel-dev
previously requested changes
Aug 10, 2026
kernel-dev
left a comment
kernel-dev
left a comment
Collaborator
There was a problem hiding this comment.
I forgot to check this before, my bad on the oversight
Comment on lines
+21
to
+23
| typedef struct { | ||
| char values[WMI_MAX_FIELDS][WMI_FIELD_LEN]; | ||
| } WmiRow; |
Collaborator
There was a problem hiding this comment.
@Mahasvan please fix this as well: you meant to do values[WMI_FIELD_LEN][WMI_MAX_FIELDS]
Mahasvan
dismissed
kernel-dev’s stale review
August 10, 2026 16:28
It's not nested in the way you think, this is the correct code 😅
Bug description: `display.py:68-73` + `README.md:122-129` +
`main.cpp:199`: 🔴 bug: EDID lookup key contradiction. The README states
`get_edid` matches on `pnp_device_id` (`MONITOR\SAMxxxx\{...}`) and
explicitly warns that passing the CCD `display_path` will return "not
found". But `display.py` passes `connector.display_path` first (the
`\\?\DISPLAY#...` path). The C++ `get_edid` (`display_info.cpp:229-230`)
does bidirectional substring matching against the SetupAPI `DevicePath`,
which is also `\\?\DISPLAY#...` format — so `display_path` is what
actually works, and the full `pnp_device_id` (`MONITOR\...`) does NOT
match (`MONITOR\` ≠ `DISPLAY#`). The C++ self-test at `main.cpp:199`
calls `fnEdid(m.pnp_device_id, ...)` which would return 0 (not found).
Fix the README to match the code, and fix `main.cpp` to call `get_edid`
with the display_path or the split segment (`SAMxxxx`) so the self-test
actually exercises EDID retrieval.
kernel-dev
approved these changes
Aug 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.