fix(x11): nk_stbi_image_to_xsurf corrupting memory when channels != bpl#954
fix(x11): nk_stbi_image_to_xsurf corrupting memory when channels != bpl#954sleeptightAnsiC wants to merge 1 commit into
Conversation
|
Hi @riri , I'm pretty sure you're the most acknowledged with all X11/Xlib/XCB-related stuff. Mind taking a look here? Something in my guts tells me nk_stbi_image_to_xsurf is simply wrong. The things it assumes and tries to do look strange in so many ways... but I kept them all unchanged. One thing I did for testing was running it under Xephyr like so... $ Xephyr :2 -screen 1000x1000x16 &
$ DISPLAY=:2 twm &
$ DISPLAY=:2 ./bin/demo...which affects the result of |
sleeptightAnsiC
force-pushed
the
fix_x11_image_corruption
branch
from
ce780cd to
ef4953b
Compare
sleeptightAnsiC
force-pushed
the
fix_x11_image_corruption
branch
from
ef4953b to
da89463
Compare
|
from #954 (comment) :
Just for the record: our version of |
sleeptightAnsiC
force-pushed
the
fix_x11_image_corruption
branch
from
da89463 to
a398938
Compare
Ah, that makes sense now - good detective work! |
nk_stbi_image_to_xsurf could corrupt memory when image channels did not match screen surface depth. I fixed it by checking DefaultDepth ahead of time and only then calling stbi_load* with previously calculated channels value. Also exported nk_xsurf_image_free with NK_API TBH, I have no idea about Xlib and don't know what I'm doing, and a lot of things in nk_stbi_image_to_xsurf was already questionable. Fixes: Immediate-Mode-UI#950 Obsoletes: Immediate-Mode-UI#952 Co-authored-by: TomJGooding <101601846+TomJGooding@users.noreply.github.com>
sleeptightAnsiC
force-pushed
the
fix_x11_image_corruption
branch
from
a398938 to
cbf8128
Compare
|
Alright, I think that's enough. Turning draft into mergable PR. It doesn't corrupt mem any more, doesn't crash, doesn't misuse stb_image, doesn't misbehave with different DefaultDepth, doesn't leak upon partial failure, etc... That's more than enough for optional feature in a demo. Our thought process is well documented here. If someone with proper Xlib knowledge will ever come and see this in the future, they'll know what we did right and/or wrong, and what to do next. |
|
I defer my review to Tom here |
3 participants
Fixes: #950
Obsoletes: #952
See commit message and linked issues for more info.
This might be totally wrong so turning it into the DRAFT.cc @TomJGooding , @RobLoach