[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [PATCH] vt82c686: fix coverity warning a
From: |
Stefan Weil |
Subject: |
Re: [Qemu-trivial] [Qemu-devel] [PATCH] vt82c686: fix coverity warning about out-of-bounds write |
Date: |
Tue, 09 Dec 2014 07:04:19 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 |
Am 09.12.2014 um 06:44 schrieb zhanghailiang:
> Refactor superio_ioport_writeb to fix the out of bounds write warning.
>
> Signed-off-by: zhanghailiang <address@hidden>
> ---
> hw/isa/vt82c686.c | 39 ++++++++++++++++++---------------------
> 1 file changed, 18 insertions(+), 21 deletions(-)
>
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index e0c235c..4516af0 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -50,13 +50,13 @@ typedef struct VT82C686BState {
> static void superio_ioport_writeb(void *opaque, hwaddr addr, uint64_t data,
> unsigned size)
> {
> - int can_write;
> SuperIOConfig *superio_conf = opaque;
>
> DPRINTF("superio_ioport_writeb address 0x%x val 0x%x\n", addr, data);
> if (addr == 0x3f0) {
> superio_conf->index = data & 0xff;
> } else {
> + int can_write = 1;
IMHO using bool instead of int would be better here (and false, true in
the following code).
> /* 0x3f1 */
> switch (superio_conf->index) {
> case 0x00 ... 0xdf:
> @@ -70,28 +70,25 @@ static void superio_ioport_writeb(void *opaque, hwaddr
> addr, uint64_t data,
> case 0xfd ... 0xff:
> can_write = 0;
> break;
> - default:
> - can_write = 1;
> -
> - if (can_write) {
> - switch (superio_conf->index) {
> - case 0xe7:
> - if ((data & 0xff) != 0xfe) {
> - DPRINTF("chage uart 1 base. unsupported yet\n");
> - }
> - break;
> - case 0xe8:
> - if ((data & 0xff) != 0xbe) {
> - DPRINTF("chage uart 2 base. unsupported yet\n");
> - }
> - break;
> -
> - default:
> - superio_conf->config[superio_conf->index] = data & 0xff;
> - }
> + case 0xe7:
> + if ((data & 0xff) != 0xfe) {
> + DPRINTF("chage uart 1 base. unsupported yet\n");
This text looks strange. Maybe a typo?
> + can_write = 0;
> + }
> + break;
> + case 0xe8:
> + if ((data & 0xff) != 0xbe) {
> + DPRINTF("chage uart 2 base. unsupported yet\n");
This text looks strange. Maybe a typo?
> + can_write = 0;
> }
> + break;
> + default:
> + break;
> +
> + }
> + if (can_write) {
> + superio_conf->config[superio_conf->index] = data & 0xff;
> }
> - superio_conf->config[superio_conf->index] = data & 0xff;
> }
> }
Regards
Stefan