[hatari-devel] Using more GCC warnings?

Eero Tamminen eerot at users.berlios.de
Sun Sep 19 01:12:49 CEST 2010


Hi,

On Saturday 18 September 2010, Nicolas Pomarède wrote:
> > So I think "-Wextra -Wno-unused-parameter" is better.
>
> I would have the same opinion as Thomas on this, I really don't like
> when code gets crippled by some gcc's specific statements.

With above, no GCC specific things are needed in code.


> > With that the only warning comes from here:
> > --------------
> > In file included from ./hatari/src/xbios.c:19:
> > ./hatari/src/includes/m68000.h: In function
> > ‘M68000_AddCyclesWithPairing’: ./hatari/src/includes/m68000.h:263:
> > warning: suggest braces around empty body in an ‘else’ statement
> > ---------------
> >
> > Maybe that could be fixed?
>
> I could add a {;} statement in the else, but as it doesn't produce
> warnings with the current flags, I'm not sure this is needed unless we
> change the flags.

That comes from warning -Wempty-body included into -Wextra.
I'm not completely sure from what kind of coding errors would
be indicated by an empty body which occurs in an if, else or do while
statement...

If you don't think it's important, that can be disabled too:
	-Wextra -Wno-unused-parameter -Wno-empty-body

After that the current code doesn't generate extra warnings.


> > Btw. there are also these:
> > $ grep CMAKE_C_FLAGS $(find -name CMakeLists.txt)
> > gui-sdl/CMakeLists.txt:set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS}
> > -Wno-write-strings") uae-cpu/CMakeLists.txt:set(CMAKE_C_FLAGS
> > "${CMAKE_C_FLAGS} -Wno-unused -Wno-sign-compare -Wno-shadow")
> >
> > Not checking for sign comparison can leave pretty hard to find bugs.
>
> Considering this is in uae cpu's emulation, I'm pretty sure this is for
> speed reason and to avoid unnecessary casts and that's there no bug
> caused by this. Trying to remove no-sign-compare and to fix all the code
> would certainly cause regression and require quite some time to fix.

I looked a bit into these warnings.  They are ones like this:
./src/uae-cpu/cpuemu.c: In function ‘op_d179_5_ff’:
./src/uae-cpu/cpuemu.c:66069: warning: comparison of promoted ~unsigned with 
unsigned

They seem to be coming from first having signed variables:
      uae_s16 src = m68k_dreg(regs, srcreg);
      uae_s16 dst = get_word(dsta);

And then doing this with it:
        SET_CFLG (((uae_u16)(~dst)) < ((uae_u16)(src)));

Which seems fine.

(There are also 8 bit variants of this.)


But it would be nice for sign-compare to be disabled in CMakeLists.txt
only for cpuemu.c which is the only C-file generating warnings, and not
for rest of C-files under uae-cpu/.


Then another thing is that the disabling GCC flags in aue-cpu and gui-sdl
subdirectories is done (unlike in the main CMakeLists.txt file) without
checking whether GCC is actually used as compiler...  Somebody should
fix that.


>> And at least on Unix, const strings go to a read-only memory section so
>> trying to write into them would cause a segfault, so it would be nice
>> to fix sdl-gui at some point too (currently it gives a huge amount of
>> warnings for that) to find out the issues at compile time instead of
>> run-time...

I'll look into this later.


	- Eero



More information about the hatari-devel mailing list