H. Peter Anvin
2013-Jan-09 22:19 UTC
[syslinux] [syslinux:firmware] vesa: Correct screencpy() prototype
On 01/07/2013 12:51 PM, syslinux-bot for Matt Fleming wrote:> Commit-ID: 15a67011987c341814533ac4d8e23c9a72dc7605 > Gitweb: http://www.syslinux.org/commit/15a67011987c341814533ac4d8e23c9a72dc7605 > Author: Matt Fleming <matt.fleming at intel.com> > AuthorDate: Mon, 7 Jan 2013 14:42:16 +0000 > Committer: Matt Fleming <matt.fleming at intel.com> > CommitDate: Mon, 7 Jan 2013 20:29:16 +0000 > > vesa: Correct screencpy() prototype > > The 'src' argument should actually be const char * because the BIOS > code performs pointer arithmetic on it. > > Signed-off-by: Matt Fleming <matt.fleming at intel.com> >Not really. The objects are uint32_t and so the prototype is correct. It should not "leak" outside the screencpy function that we play ugly tricks with these pointers; that is better handled by an internal cast. -hpa
Matt Fleming
2013-Jan-10 11:41 UTC
[syslinux] [syslinux:firmware] vesa: Correct screencpy() prototype
On Wed, 2013-01-09 at 14:19 -0800, H. Peter Anvin wrote:> On 01/07/2013 12:51 PM, syslinux-bot for Matt Fleming wrote: > > Commit-ID: 15a67011987c341814533ac4d8e23c9a72dc7605 > > Gitweb: http://www.syslinux.org/commit/15a67011987c341814533ac4d8e23c9a72dc7605 > > Author: Matt Fleming <matt.fleming at intel.com> > > AuthorDate: Mon, 7 Jan 2013 14:42:16 +0000 > > Committer: Matt Fleming <matt.fleming at intel.com> > > CommitDate: Mon, 7 Jan 2013 20:29:16 +0000 > > > > vesa: Correct screencpy() prototype > > > > The 'src' argument should actually be const char * because the BIOS > > code performs pointer arithmetic on it. > > > > Signed-off-by: Matt Fleming <matt.fleming at intel.com> > > > > Not really. > > The objects are uint32_t and so the prototype is correct. It should not > "leak" outside the screencpy function that we play ugly tricks with > these pointers; that is better handled by an internal cast.Does this look better? --->From bf9500946a2e9d62c80451eaa5e5a39f6f0876a6 Mon Sep 17 00:00:00 2001From: Matt Fleming <matt.fleming at intel.com> Date: Thu, 10 Jan 2013 11:22:56 +0000 Subject: [PATCH] Revert "vesa: Correct screencpy() prototype" This reverts commit 15a67011987c341814533ac4d8e23c9a72dc7605, because as Peter points out, The objects are uint32_t and so the prototype is correct. It should not "leak" outside the screencpy function that we play ugly tricks with these pointers; that is better handled by an internal cast. So do the necessary casting inside bios_vesacon_screencpy(). Signed-off-by: Matt Fleming <matt.fleming at intel.com> --- com32/include/syslinux/firmware.h | 2 +- com32/lib/sys/vesa/screencpy.c | 4 ++-- core/bios.c | 7 ++++--- efi/vesa.c | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/com32/include/syslinux/firmware.h b/com32/include/syslinux/firmware.h index 268e0c8..13ba159 100644 --- a/com32/include/syslinux/firmware.h +++ b/com32/include/syslinux/firmware.h @@ -33,7 +33,7 @@ struct win_info; struct vesa_ops { int (*set_mode)(struct vesa_info *, int *, int *, enum vesa_pixel_format *); - void (*screencpy)(size_t, const char *, size_t, struct win_info *); + void (*screencpy)(size_t, const uint32_t *, size_t, struct win_info *); int (*font_query)(uint8_t **); }; diff --git a/com32/lib/sys/vesa/screencpy.c b/com32/lib/sys/vesa/screencpy.c index 5c6d915..d78109b 100644 --- a/com32/lib/sys/vesa/screencpy.c +++ b/com32/lib/sys/vesa/screencpy.c @@ -70,8 +70,8 @@ void __vesacon_copy_to_screen(size_t dst, const uint32_t * src, size_t npixels) { size_t bytes = npixels * __vesacon_bytes_per_pixel; char rowbuf[bytes + 4] __aligned(4); - const char *s; + const uint32_t *s; - s = (const char *)__vesacon_format_pixels(rowbuf, src, npixels); + s = (const uint32_t *)__vesacon_format_pixels(rowbuf, src, npixels); firmware->vesa->screencpy(dst, s, bytes, &wi); } diff --git a/core/bios.c b/core/bios.c index d0ceffc..1becd28 100644 --- a/core/bios.c +++ b/core/bios.c @@ -414,13 +414,14 @@ static void set_window_pos(struct win_info *wi, size_t win_pos) __intcall(0x10, &ireg, NULL); } -static void bios_vesacon_screencpy(size_t dst, const char * src, +static void bios_vesacon_screencpy(size_t dst, const uint32_t * src, size_t bytes, struct win_info *wi) { size_t win_pos, win_off; size_t win_size = wi->win_size; size_t omask = win_size - 1; char *win_base = wi->win_base; + const char *s = (const char *)src; size_t l; while (bytes) { @@ -431,10 +432,10 @@ static void bios_vesacon_screencpy(size_t dst, const char * src, set_window_pos(wi, win_pos); l = min(bytes, win_size - win_off); - memcpy(win_base + win_off, src, l); + memcpy(win_base + win_off, s, l); bytes -= l; - src += l; + s += l; dst += l; } } diff --git a/efi/vesa.c b/efi/vesa.c index 87308fd..e3e9e0f 100644 --- a/efi/vesa.c +++ b/efi/vesa.c @@ -274,7 +274,7 @@ exit: return err; } -static void efi_vesacon_screencpy(size_t dst, const char *s, +static void efi_vesacon_screencpy(size_t dst, const uint32_t *s, size_t bytes, struct win_info *wi) { size_t win_off; -- 1.7.11.7 -- Matt Fleming, Intel Open Source Technology Center