* [Qemu-devel] [PATCH 06/15] Use range_covers_byte
@ 2010-09-05 15:06 Blue Swirl
2010-09-05 18:03 ` [Qemu-devel] " Michael S. Tsirkin
0 siblings, 1 reply; 3+ messages in thread
From: Blue Swirl @ 2010-09-05 15:06 UTC (permalink / raw)
To: qemu-devel
Use range_covers_byte() instead of comparisons.
This also fixes some warnings with GCC flag -Wtype-limits.
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
hw/omap1.c | 21 +++++++++++++++------
hw/sm501.c | 5 +++--
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/hw/omap1.c b/hw/omap1.c
index 06370b6..2dd62ec 100644
--- a/hw/omap1.c
+++ b/hw/omap1.c
@@ -26,6 +26,7 @@
/* We use pc-style serial ports. */
#include "pc.h"
#include "blockdev.h"
+#include "range.h"
/* Should signal the TCMI/GPMC */
uint32_t omap_badwidth_read8(void *opaque, target_phys_addr_t addr)
@@ -3669,37 +3670,45 @@ static const struct dma_irq_map omap1_dma_irq_map[] = {
static int omap_validate_emiff_addr(struct omap_mpu_state_s *s,
target_phys_addr_t addr)
{
- return addr >= OMAP_EMIFF_BASE && addr < OMAP_EMIFF_BASE + s->sdram_size;
+ return range_covers_byte(OMAP_EMIFF_BASE,
+ OMAP_EMIFF_BASE + s->sdram_size - OMAP_EMIFF_BASE,
+ addr);
}
static int omap_validate_emifs_addr(struct omap_mpu_state_s *s,
target_phys_addr_t addr)
{
- return addr >= OMAP_EMIFS_BASE && addr < OMAP_EMIFF_BASE;
+ return range_covers_byte(OMAP_EMIFS_BASE, OMAP_EMIFF_BASE -
OMAP_EMIFS_BASE,
+ addr);
}
static int omap_validate_imif_addr(struct omap_mpu_state_s *s,
target_phys_addr_t addr)
{
- return addr >= OMAP_IMIF_BASE && addr < OMAP_IMIF_BASE + s->sram_size;
+ return range_covers_byte(OMAP_IMIF_BASE,
+ OMAP_IMIF_BASE + s->sram_size - OMAP_IMIF_BASE,
+ addr);
}
static int omap_validate_tipb_addr(struct omap_mpu_state_s *s,
target_phys_addr_t addr)
{
- return addr >= 0xfffb0000 && addr < 0xffff0000;
+ return range_covers_byte(0xfffb0000, 0xffff0000 - 0xfffb0000, addr);
}
static int omap_validate_local_addr(struct omap_mpu_state_s *s,
target_phys_addr_t addr)
{
- return addr >= OMAP_LOCALBUS_BASE && addr < OMAP_LOCALBUS_BASE + 0x1000000;
+ return range_covers_byte(OMAP_LOCALBUS_BASE,
+ OMAP_LOCALBUS_BASE + 0x1000000 -
+ OMAP_LOCALBUS_BASE,
+ addr);
}
static int omap_validate_tipb_mpui_addr(struct omap_mpu_state_s *s,
target_phys_addr_t addr)
{
- return addr >= 0xe1010000 && addr < 0xe1020004;
+ return range_covers_byte(0xe1010000, 0xe1020004 - 0xe1010000, addr);
}
struct omap_mpu_state_s *omap310_mpu_init(unsigned long sdram_size,
diff --git a/hw/sm501.c b/hw/sm501.c
index 8e6932d..705e0a5 100644
--- a/hw/sm501.c
+++ b/hw/sm501.c
@@ -29,6 +29,7 @@
#include "devices.h"
#include "sysbus.h"
#include "qdev-addr.h"
+#include "range.h"
/*
* Status: 2010/05/07
@@ -814,7 +815,7 @@ static uint32_t sm501_palette_read(void *opaque,
target_phys_addr_t addr)
/* TODO : consider BYTE/WORD access */
/* TODO : consider endian */
- assert(0 <= addr && addr < 0x400 * 3);
+ assert(range_covers_byte(0, 0x400 * 3, addr));
return *(uint32_t*)&s->dc_palette[addr];
}
@@ -828,7 +829,7 @@ static void sm501_palette_write(void *opaque,
/* TODO : consider BYTE/WORD access */
/* TODO : consider endian */
- assert(0 <= addr && addr < 0x400 * 3);
+ assert(range_covers_byte(0, 0x400 * 3, addr));
*(uint32_t*)&s->dc_palette[addr] = value;
}
--
1.6.2.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [Qemu-devel] Re: [PATCH 06/15] Use range_covers_byte
2010-09-05 15:06 [Qemu-devel] [PATCH 06/15] Use range_covers_byte Blue Swirl
@ 2010-09-05 18:03 ` Michael S. Tsirkin
2010-09-05 19:46 ` Blue Swirl
0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2010-09-05 18:03 UTC (permalink / raw)
To: Blue Swirl; +Cc: qemu-devel
On Sun, Sep 05, 2010 at 03:06:07PM +0000, Blue Swirl wrote:
> Use range_covers_byte() instead of comparisons.
>
> This also fixes some warnings with GCC flag -Wtype-limits.
>
> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
To me (not a native english speaker)
this comment implies that there's a bugfix here. Is there?
> ---
> hw/omap1.c | 21 +++++++++++++++------
> hw/sm501.c | 5 +++--
> 2 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/hw/omap1.c b/hw/omap1.c
> index 06370b6..2dd62ec 100644
> --- a/hw/omap1.c
> +++ b/hw/omap1.c
> @@ -26,6 +26,7 @@
> /* We use pc-style serial ports. */
> #include "pc.h"
> #include "blockdev.h"
> +#include "range.h"
>
> /* Should signal the TCMI/GPMC */
> uint32_t omap_badwidth_read8(void *opaque, target_phys_addr_t addr)
> @@ -3669,37 +3670,45 @@ static const struct dma_irq_map omap1_dma_irq_map[] = {
> static int omap_validate_emiff_addr(struct omap_mpu_state_s *s,
> target_phys_addr_t addr)
> {
> - return addr >= OMAP_EMIFF_BASE && addr < OMAP_EMIFF_BASE + s->sdram_size;
> + return range_covers_byte(OMAP_EMIFF_BASE,
> + OMAP_EMIFF_BASE + s->sdram_size - OMAP_EMIFF_BASE,
> + addr);
same as
return range_covers_byte(OMAP_EMIFF_BASE,
s->sdram_size,
addr);
> }
>
> static int omap_validate_emifs_addr(struct omap_mpu_state_s *s,
> target_phys_addr_t addr)
> {
> - return addr >= OMAP_EMIFS_BASE && addr < OMAP_EMIFF_BASE;
> + return range_covers_byte(OMAP_EMIFS_BASE, OMAP_EMIFF_BASE -
> OMAP_EMIFS_BASE,
> + addr);
> }
>
> static int omap_validate_imif_addr(struct omap_mpu_state_s *s,
> target_phys_addr_t addr)
> {
> - return addr >= OMAP_IMIF_BASE && addr < OMAP_IMIF_BASE + s->sram_size;
> + return range_covers_byte(OMAP_IMIF_BASE,
> + OMAP_IMIF_BASE + s->sram_size - OMAP_IMIF_BASE,
> + addr);
same as
return range_covers_byte(OMAP_IMIF_BASE,
s->sram_size,
addr);
?
> }
>
> static int omap_validate_tipb_addr(struct omap_mpu_state_s *s,
> target_phys_addr_t addr)
> {
> - return addr >= 0xfffb0000 && addr < 0xffff0000;
> + return range_covers_byte(0xfffb0000, 0xffff0000 - 0xfffb0000, addr);
> }
>
repeating the constant 0xfffb0000 is a bit ugly ... give them names?
> static int omap_validate_local_addr(struct omap_mpu_state_s *s,
> target_phys_addr_t addr)
> {
> - return addr >= OMAP_LOCALBUS_BASE && addr < OMAP_LOCALBUS_BASE + 0x1000000;
> + return range_covers_byte(OMAP_LOCALBUS_BASE,
> + OMAP_LOCALBUS_BASE + 0x1000000 -
> + OMAP_LOCALBUS_BASE,
> + addr);
Same as
return range_covers_byte(OMAP_LOCALBUS_BASE,
0x1000000,
addr);
?
> }
>
> static int omap_validate_tipb_mpui_addr(struct omap_mpu_state_s *s,
> target_phys_addr_t addr)
> {
> - return addr >= 0xe1010000 && addr < 0xe1020004;
> + return range_covers_byte(0xe1010000, 0xe1020004 - 0xe1010000, addr);
> }
repeating the constants is a bit ugly ... give them names?
>
> struct omap_mpu_state_s *omap310_mpu_init(unsigned long sdram_size,
> diff --git a/hw/sm501.c b/hw/sm501.c
> index 8e6932d..705e0a5 100644
> --- a/hw/sm501.c
> +++ b/hw/sm501.c
> @@ -29,6 +29,7 @@
> #include "devices.h"
> #include "sysbus.h"
> #include "qdev-addr.h"
> +#include "range.h"
>
> /*
> * Status: 2010/05/07
> @@ -814,7 +815,7 @@ static uint32_t sm501_palette_read(void *opaque,
> target_phys_addr_t addr)
> /* TODO : consider BYTE/WORD access */
> /* TODO : consider endian */
>
> - assert(0 <= addr && addr < 0x400 * 3);
> + assert(range_covers_byte(0, 0x400 * 3, addr));
> return *(uint32_t*)&s->dc_palette[addr];
> }
>
> @@ -828,7 +829,7 @@ static void sm501_palette_write(void *opaque,
> /* TODO : consider BYTE/WORD access */
> /* TODO : consider endian */
>
> - assert(0 <= addr && addr < 0x400 * 3);
> + assert(range_covers_byte(0, 0x400 * 3, addr));
> *(uint32_t*)&s->dc_palette[addr] = value;
> }
>
> --
> 1.6.2.4
^ permalink raw reply [flat|nested] 3+ messages in thread
* [Qemu-devel] Re: [PATCH 06/15] Use range_covers_byte
2010-09-05 18:03 ` [Qemu-devel] " Michael S. Tsirkin
@ 2010-09-05 19:46 ` Blue Swirl
0 siblings, 0 replies; 3+ messages in thread
From: Blue Swirl @ 2010-09-05 19:46 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel
On Sun, Sep 5, 2010 at 6:03 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Sun, Sep 05, 2010 at 03:06:07PM +0000, Blue Swirl wrote:
>> Use range_covers_byte() instead of comparisons.
>>
>> This also fixes some warnings with GCC flag -Wtype-limits.
>>
>> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
>
> To me (not a native english speaker)
> this comment implies that there's a bugfix here. Is there?
No, perhaps 'avoids warnings' would be a better description.
>
>> ---
>> hw/omap1.c | 21 +++++++++++++++------
>> hw/sm501.c | 5 +++--
>> 2 files changed, 18 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/omap1.c b/hw/omap1.c
>> index 06370b6..2dd62ec 100644
>> --- a/hw/omap1.c
>> +++ b/hw/omap1.c
>> @@ -26,6 +26,7 @@
>> /* We use pc-style serial ports. */
>> #include "pc.h"
>> #include "blockdev.h"
>> +#include "range.h"
>>
>> /* Should signal the TCMI/GPMC */
>> uint32_t omap_badwidth_read8(void *opaque, target_phys_addr_t addr)
>> @@ -3669,37 +3670,45 @@ static const struct dma_irq_map omap1_dma_irq_map[] = {
>> static int omap_validate_emiff_addr(struct omap_mpu_state_s *s,
>> target_phys_addr_t addr)
>> {
>> - return addr >= OMAP_EMIFF_BASE && addr < OMAP_EMIFF_BASE + s->sdram_size;
>> + return range_covers_byte(OMAP_EMIFF_BASE,
>> + OMAP_EMIFF_BASE + s->sdram_size - OMAP_EMIFF_BASE,
>> + addr);
>
>
> same as
>
> return range_covers_byte(OMAP_EMIFF_BASE,
> s->sdram_size,
> addr);
Ok. I made the conversion a bit too mechanically.
>
>> }
>>
>> static int omap_validate_emifs_addr(struct omap_mpu_state_s *s,
>> target_phys_addr_t addr)
>> {
>> - return addr >= OMAP_EMIFS_BASE && addr < OMAP_EMIFF_BASE;
>> + return range_covers_byte(OMAP_EMIFS_BASE, OMAP_EMIFF_BASE -
>> OMAP_EMIFS_BASE,
>> + addr);
>> }
>>
>> static int omap_validate_imif_addr(struct omap_mpu_state_s *s,
>> target_phys_addr_t addr)
>> {
>> - return addr >= OMAP_IMIF_BASE && addr < OMAP_IMIF_BASE + s->sram_size;
>> + return range_covers_byte(OMAP_IMIF_BASE,
>> + OMAP_IMIF_BASE + s->sram_size - OMAP_IMIF_BASE,
>> + addr);
>
>
> same as
> return range_covers_byte(OMAP_IMIF_BASE,
> s->sram_size,
> addr);
> ?
>
>> }
>>
>> static int omap_validate_tipb_addr(struct omap_mpu_state_s *s,
>> target_phys_addr_t addr)
>> {
>> - return addr >= 0xfffb0000 && addr < 0xffff0000;
>> + return range_covers_byte(0xfffb0000, 0xffff0000 - 0xfffb0000, addr);
>> }
>>
>
> repeating the constant 0xfffb0000 is a bit ugly ... give them names?
I agree, but I'd rather not fix all problems with this patch set.
>
>> static int omap_validate_local_addr(struct omap_mpu_state_s *s,
>> target_phys_addr_t addr)
>> {
>> - return addr >= OMAP_LOCALBUS_BASE && addr < OMAP_LOCALBUS_BASE + 0x1000000;
>> + return range_covers_byte(OMAP_LOCALBUS_BASE,
>> + OMAP_LOCALBUS_BASE + 0x1000000 -
>> + OMAP_LOCALBUS_BASE,
>> + addr);
>
>
> Same as
> return range_covers_byte(OMAP_LOCALBUS_BASE,
> 0x1000000,
> addr);
>
> ?
>
>> }
>>
>> static int omap_validate_tipb_mpui_addr(struct omap_mpu_state_s *s,
>> target_phys_addr_t addr)
>> {
>> - return addr >= 0xe1010000 && addr < 0xe1020004;
>> + return range_covers_byte(0xe1010000, 0xe1020004 - 0xe1010000, addr);
>> }
>
> repeating the constants is a bit ugly ... give them names?
>
>>
>> struct omap_mpu_state_s *omap310_mpu_init(unsigned long sdram_size,
>> diff --git a/hw/sm501.c b/hw/sm501.c
>> index 8e6932d..705e0a5 100644
>> --- a/hw/sm501.c
>> +++ b/hw/sm501.c
>> @@ -29,6 +29,7 @@
>> #include "devices.h"
>> #include "sysbus.h"
>> #include "qdev-addr.h"
>> +#include "range.h"
>>
>> /*
>> * Status: 2010/05/07
>> @@ -814,7 +815,7 @@ static uint32_t sm501_palette_read(void *opaque,
>> target_phys_addr_t addr)
>> /* TODO : consider BYTE/WORD access */
>> /* TODO : consider endian */
>>
>> - assert(0 <= addr && addr < 0x400 * 3);
>> + assert(range_covers_byte(0, 0x400 * 3, addr));
>> return *(uint32_t*)&s->dc_palette[addr];
>> }
>>
>> @@ -828,7 +829,7 @@ static void sm501_palette_write(void *opaque,
>> /* TODO : consider BYTE/WORD access */
>> /* TODO : consider endian */
>>
>> - assert(0 <= addr && addr < 0x400 * 3);
>> + assert(range_covers_byte(0, 0x400 * 3, addr));
>> *(uint32_t*)&s->dc_palette[addr] = value;
>> }
>>
>> --
>> 1.6.2.4
>
Thanks for the review.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-09-05 19:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-05 15:06 [Qemu-devel] [PATCH 06/15] Use range_covers_byte Blue Swirl
2010-09-05 18:03 ` [Qemu-devel] " Michael S. Tsirkin
2010-09-05 19:46 ` Blue Swirl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).