Fix numeric keysym parsing #357
Merged
Conversation
This file contains 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
Closed
wismill
added
the
compile-keymap
bluetech
reviewed
Jul 3, 2023
| @@ -656,11 +656,25 @@ ExprResolveKeySym(struct xkb_context *ctx, const ExprDef *expr, | |||
| if (!ExprResolveInteger(ctx, expr, &val)) | |||
| return false; | |||
|
|
|||
| if (val < 0 || val >= 10) | |||
There was a problem hiding this comment.
This looks like it regressed in 2e4933c compared to xkbcomp...
|
@bluetech Commit split! |
whot
approved these changes
Jul 4, 2023
|
Rebased. I added the constant I also added much more test related to keysyms, to reflect the current implementation. |
bluetech
reviewed
Jul 4, 2023
Keysyms are 32-bit integers with the 3 most significant bits always set to zero. See: Appendix A “KEYSYM Encoding” of the X Window System Protocol at https://www.x.org/releases/current/doc/xproto/x11protocol.html#keysym_encoding. Add a new constants XKB_KEYSYM_MIN and XKB_KEYSYM_MAX to make the interval of valid keysyms more obvious in the code.
When parsing hexadecimal keysym using `xkb_keysym_from_name`, the result is limited by `parse_keysym_hex` to 0xffffffff, but the maximum keysym is XKB_MAX_KEYSYM, i.e. 0x1fffffff. Fixed by adding an upper bound.
In `parser.y`, a numeric keysym is parsed by formatting it in its hexadecimal form then parsed as a keysym name. This is convoluted. Fixed by checking directly the upper bound.
`ExprResolveKeySym` in `expr.c` does not parse non-digit numeric keysyms. Fixed by checking upper bound; also add warning messages.
- Add a keymap test with decimal and hexadecimal keysyms. - Reorganize code in `test/keysym.c` by parsing type: name, Unicode and hexadecimal. - Add more tests for edge cases. In particular: - test decimal format (currently not supported); - test the Unicode and hexadecimal ranges more thoroughly; - test with wrong case without the XKB_KEYSYM_CASE_INSENSITIVE flag; - test surrounding spaces. - Document the tests.
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.
While working on #332, I realized that numeric keysyms parsing has some issues. Let’s deal with it in this separate PR.
This PR fixes some edges cases and improve tests.