Re: [Qemu-devel] [PATCH 1/5] Suppress some gcc warnings with -Wtype-limits

[andrzej zaborowski]

On 4 September 2010 16:17, Blue Swirl <address@hidden> wrote:
> Add various casts, adjust types etc. to make the warnings with
> gcc flag -Wtype-limits disappear.
>
> Signed-off-by: Blue Swirl <address@hidden>
> ---
>  block/blkdebug.c      |    2 +-
>  hw/mips_fulong2e.c    |    2 +-
>  hw/omap1.c            |    2 +-
>  hw/ppc405_boards.c    |   23 +++++++++++++----------
>  hw/ppc_newworld.c     |    3 ++-
>  hw/ppc_prep.c         |    3 ++-
>  hw/pxa2xx.c           |    2 +-
>  hw/sm501.c            |    4 ++--
>  linux-user/flatload.c |    3 ++-
>  linux-user/mmap.c     |    2 +-
>  linux-user/syscall.c  |   20 +++++++++++++-------
>  11 files changed, 39 insertions(+), 27 deletions(-)
>
> diff --git a/block/blkdebug.c b/block/blkdebug.c
> index 2a63df9..b5083bc 100644
> --- a/block/blkdebug.c
> +++ b/block/blkdebug.c
> @@ -439,7 +439,7 @@ static void blkdebug_debug_event(BlockDriverState
> *bs, BlkDebugEvent event)
>     struct BlkdebugRule *rule;
>     BlkdebugVars old_vars = s->vars;
>
> -    if (event < 0 || event >= BLKDBG_EVENT_MAX) {
> +    if ((unsigned int)event >= BLKDBG_EVENT_MAX) {
>         return;
>     }
>
> diff --git a/hw/mips_fulong2e.c b/hw/mips_fulong2e.c
> index cbe7156..ac82067 100644
> --- a/hw/mips_fulong2e.c
> +++ b/hw/mips_fulong2e.c
> @@ -258,7 +258,7 @@ static void mips_fulong2e_init(ram_addr_t
> ram_size, const char *boot_device,
>  {
>     char *filename;
>     unsigned long ram_offset, bios_offset;
> -    unsigned long bios_size;
> +    long bios_size;
>     int64_t kernel_entry;
>     qemu_irq *i8259;
>     qemu_irq *cpu_exit_irq;
> diff --git a/hw/omap1.c b/hw/omap1.c
> index 06370b6..b04aa80 100644
> --- a/hw/omap1.c
> +++ b/hw/omap1.c
> @@ -3675,7 +3675,7 @@ static int omap_validate_emiff_addr(struct
> omap_mpu_state_s *s,
>  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 addr < OMAP_EMIFF_BASE;

I think this only doesn't break the code because it relies on a
specific value for that #define, and if the value is changed, the
check has to be re-added.  Same applies to other modifications in this
patch.

Most of the gcc warnings should not be enabled by default and I think
this is one of them.  They make you do some really strange things in
the code just to avoid using one construct of your programming
language (fully valid in the language), and in the end result in bugs
of other types.  It's probably ok to enable them once to see if they
hint on any bugs, but if enabled by default, they'll just cause other
types of bugs.

There are some projects that avoid using strcat for example, because,
if misused, it can cause crashes.  There's even a pretty big project
by Nokia that does not use malloc(), because again, "malloc can cause
memory leaks".  They just allocate all memory statically, and it
works.. but is a pain to work with, and the program requires much more
memory than needed so it doesn't run on some embedded devices.  I
wouldn't want to go this way.

Cheers