Hi,
Thanks for reviewing. I did some changes on my patch. I also closed the
PR on salsa because we are using debian bug tracker instead of gitlab
pull requests.
On 2022/5/30 03:49, Samuel Thibault wrote:
Then is it not possible to exchange the patch order? So that whatever
the number of patches being applied, things work completely.
The two patch is tightly related. I guess why the original code leak a
font mmap intentionally is just to avoid the
font-unmap-during-drawing-will-cause-crash problem. The
async-signal-safefy patch will defer font reloading to main loop to
avoid this problem (and fix other problems like calling malloc() in
signal handlers which is very dangerous).
I updated my patch. The first patch (Better font reload handling) will
comment out the call to bogl_munmap_font(), avoiding crash but still
have a leak, just like the original code. So if someone only applies the
first patch, it will work as before (not crash but still leak memory).
The second patch (Fix incorrect signal handling) will re-enable the call
previously commented out in first patch. So if someone want to apply the
second patch, he/she must apply the first patch first, make dependency
more clear. So, things are always working if (a) only first patch is
applied, or (b) both first and second patch are applied, or (c) non of
these two patch are applied, and (d) it is not possible to only apply
second patch because the patch command will complain about conflicts.
While at it, please use MAP_FAILED rather than the hardcoded (void*)-1
Fixed. :-)
This looks unrelated and bogus?
This is because I want to maintain a constant look of signal handlers.
The reason behind it is there is no need to call signal() to install
handler again because glibc's signal() now provide BSD semantics per man
page of signal(). I updated my patch and use a sigaction wrapper to
address this explicitly, and this will make the code more portable.
Do you have a reference that documents this?
Unfortunately there is no document about this, mainly because this is a
workaround for buggy drivers. I updated comments in code to address this
more explicitly.
MAP_FAILED here as well.
Fixed. :-)
Please make these an inline function rather than copy/pasting them from
bogl_mmap_font.
Fixed. :-)
Best Regards,
Zhang Boyang
