Repository navigation
CPython: struct fields take dicts, and a wrong type raises instead of segfaulting - #23
Merged
Merged
Conversation
…faults The by-value macro dereferenced mp_write_ptr_*'s NULL whenever the value wasn't an instance of the struct's type. It now takes a dict the way the type's constructor does, as MicroPython does, and for anything else sets TypeError and yields a zeroed scratch copy that is never kept: a setter puts the struct back as it was and returns -1, and function wrappers already return NULL on PyErr_Occurred(). Fixes #21
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.
Fixes #21.
On CPython,
lv.image_dsc_t({"header": {"w": 4}}),dsc.header = {"w": 4}anddsc.header = 5all segfaulted. The by-value macromp_write_<struct>(value)was*(T*)mp_write_ptr_<struct>(value), and the pointer function returns NULL for anything that isn't that struct type.What it does now (CPython emitter only; MicroPython and CircuitPython output is unchanged):
dsc.header = {"w": 7}leavesw=7, h=0on each.TypeErrorand yields a zeroed scratch copy, so nothing dereferences NULL. Function wrappers already returned NULL onPyErr_Occurred(). Setters now do the same, and they first put the struct back as it was, because the placeholder has already been stored by then. The first version of this missed that, and the smoke test caught it.What I ran:
pytest tests: 111 passed. The two new generator tests fail on main../regenerate_all.sh --check: all generated artifacts match.lvgl_python.c, on Linux with Python 3.12.tools/test_lvgl_smoke.py, with the newtest_struct_value_from_dict, printed "All LVGL smoke tests passed.", and lvgl-python's unit tests passed (6 OK). The same smoke test dumps core on the current 9.5.47 build.4 3 12, and5,Noneand"x"each raiseTypeError.SyntaxError.Not run: the Windows (MSVC) and Pyodide builds. The new C uses only the Python 3.11 C API and standard C.
The fix reaches users when lvgl-python syncs past this commit and a release is cut.