Hello,
Zhang Boyang, le dim. 29 mai 2022 14:23:19 +0800, a ecrit:
> Subject: [PATCH v2 1/8] Better quit handling
That looks good, applied!
> From 302952c64952197d694039358df407ec3ffa418a Mon Sep 17 00:00:00 2001
> From: Zhang Boyang <zhangboyang...@gmail.com>
> Date: Mon, 23 May 2022 14:10:52 +0800
> Subject: [PATCH v2 2/8] Better font reload handling
>
> Previous font reload code will leak a mmap on each reload. This patch
> adds the ability to munmap old font after reload. However, this also
> introduces a bug, if font reload is triggered while drawing in progress,
> after signal handler returns, the drawing code will continue to use old
> font which has been freed, causing crash. This will be fixed in next
> patch.
Then is it not possible to exchange the patch order? So that whatever
the number of patches being applied, things work completely.
> - void *f;
> - struct bogl_font *font;
> + void *f = (void *)-1;
[...]
> f = mmap(0, buf.st_size, PROT_READ, MAP_SHARED, fd, 0);
> if (f == (void *)-1)
While at it, please use MAP_FAILED rather than the hardcoded (void*)-1
Otherwise it looks good.
> From 4b2e0651edf19cab4b162c686c21e81682c854a0 Mon Sep 17 00:00:00 2001
> From: Zhang Boyang <zhangboyang...@gmail.com>
> Date: Tue, 24 May 2022 12:58:01 +0800
> Subject: [PATCH v2 3/8] Fix incorrect signal handling
>
> There are problems in signal handlers. Signal handlers must not call any
> non-async-signal-safe functions, and they must save-restore errno if
> errno is modified inside signal handlers. This patch fixes these
> problems by deferring real tasks to main loop. This also fixes crashes
> caused by font reloading which was mentioned in previous patch.
> diff --git a/bterm.c b/bterm.c
> index 28ccb53..b0a1189 100644
> --- a/bterm.c
> +++ b/bterm.c
> @@ -160,7 +160,6 @@ void sigchld(int sig)
> quit_status = status;
> quit = 1;
> }
> - signal(SIGCHLD, sigchld);
> errno = errsv;
> }
>
This looks unrelated and bogus?
Otherwise it looks good.
> From e7fe989fbbda4acfd9971604b7ffa84899cb343a Mon Sep 17 00:00:00 2001
> From: Zhang Boyang <zhangboyang...@gmail.com>
> Date: Mon, 23 May 2022 22:07:37 +0800
> Subject: [PATCH v2 4/8] Use ioctl(FBIOPAN_DISPLAY) to update screen after
> drawing
>
> Some framebuffer driver like i915 need explicit notify after drawing,
> otherwise modifications in graphics buffer will not be reflected on
> screen, so use FBIOPAN_DISPLAY to do this notify.
Do you have a reference that documents this?
Otherwise it looks good.
> From 04d88a6ff7114750c67cdb58e07122bb430763a9 Mon Sep 17 00:00:00 2001
> From: Zhang Boyang <zhangboyang...@gmail.com>
> Date: Tue, 24 May 2022 23:49:31 +0800
> Subject: [PATCH v2 5/8] Font scaler
[...]
> +bgf_scale_inline(struct bogl_bgf *bgf, int scale)
> +{
> + void *new_f = (void *)-1;
> + off_t new_size;
> + struct bogl_font new_font;
> +
> + /* old_size*pow(scale,2) should enough and have some waste here */
> + new_size = bgf->size * scale * scale;
> +
> + /* Allocate new memory */
> + new_f = mmap(0, new_size, PROT_READ | PROT_WRITE, MAP_PRIVATE |
> MAP_ANONYMOUS, -1, 0);
> + if (new_f == (void *)-1) {
MAP_FAILED here as well.
> + /* If memory allocation failed, skip scaling silently */
> + goto fail;
> + }
> +
> + /* Copy old font data to new memory */
> + memcpy(new_f, bgf->f, bgf->size);
> +
> + /* Set font metadata */
> + struct bogl_font *font = &new_font;
> + memcpy(font, new_f + 4, sizeof(*font));
> + font->name = ((void *)font->name - (void *)0) + new_f;
> + font->offset = ((void *)font->offset - (void *)0) + new_f;
> + font->index = ((void *)font->index - (void *)0) + new_f;
> + font->content = ((void *)font->content - (void *)0) + new_f;
Please make these an inline function rather than copy/pasting them from
bogl_mmap_font.
Otherwise it looks good.
> Subject: [PATCH v2 6/8] Fix character occasionally disappears from right edge
> of screen
That looks good, applied!
> Subject: [PATCH v2 7/8] Fix out-of-bound read in tcfb's cmap_lookup()
That looks good, applied!
> Subject: [PATCH v2 8/8] Fix rightmost pixel of each line not cleared by
> bogl_vga16_text()
That looks good, applied!
And thanks!
Samuel
