search

klange / toaruos

A completely-from-scratch hobby operating system: bootloader, kernel, drivers, C library, and userspace including a composited graphical UI, dynamic linker, syntax-highlighting text editor, network stack, etc.
https://toaruos.org/
University of Illinois/NCSA Open Source License
6.12k stars 485 forks source link

Yutani: checking icon for NULL too late #276

Open mxlgv opened 1 year ago

mxlgv commented 1 year ago

Yutani doesn't check icon for NULL, probably causing applications to crash (Segmentation fault) when setting the caption in SDL like this: 

SDL_WM_SetCaption("Wolfenstein 3D", NULL);

https://github.com/klange/toaruos/blob/abe66fb45ba60c26a927023706e25de34493ed98/lib/yutani.c#L809

klange commented 1 year ago

Despite a null check later in the function, yutani_window_advertise_icon is not intended to accept NULL for the icon; clients should use yutani_window_advertise instead. The bug lies in the SDL backend: TOARU_SetCaption should be choosing yutani_window_advertise if icon is NULL. I would transfer this issue to the SDL fork, but I don't seem to have issues enabled for that repository.

mxlgv commented 1 year ago

In that case, this if is redundant: https://github.com/klange/toaruos/blob/abe66fb45ba60c26a927023706e25de34493ed98/lib/yutani.c#L816

klange commented 1 year ago

Yeah. Maybe it should support a NULL icon... and it should probably also support a NULL name, which it fails to do in the same way... :\

mxlgv commented 1 year ago

Will you fix it? In my opinion, the fix is ​​quite obvious.

klange commented 1 year ago

I've got a fix pending on the SDL side. Still thinking about what I want to do here. Falling back to the no-icon form is likely the right choice, though I am also thinking about what should be done in both functions if the title is NULL - perhaps a signal to remove the window from the advertised window list.