[PATCH] YABEL compilation warnings

[Myles Watson]

This patch removes most of the rest of the compilation warnings for me.

1. Move run_bios prototype to device.h
2. Use time.h for get_time()
3. Move read_io and write_io to io.c and make them static
4. Make a couple of functions static in interrupt.c
5. Refactor a cast from char[] to u64 to get rid of potential alignment
problems and a warning

The only ones left are "unused" warnings.

I think we should get rid of that warning, since we conditionally call
functions based on debugging and various config variables.  Is there a case
where it helps enough to justify all the warnings?

This next part isn't part of the patch, but applying it makes qemu compile
with yabel (with and without debugging).

Index: Makefile
===================================================================
--- Makefile    (revision 5186)
+++ Makefile    (working copy)
@@ -239,7 +239,7 @@
 CFLAGS = $(INCLUDES) -Os -nostdinc
 CFLAGS += -nostdlib -Wall -Wundef -Wstrict-prototypes -Wmissing-prototypes
 CFLAGS += -Wwrite-strings -Wredundant-decls -Wno-trigraphs
-CFLAGS += -Wstrict-aliasing -Wshadow
+CFLAGS += -Wstrict-aliasing -Wshadow -Wno-unused
 ifeq ($(CONFIG_WARNINGS_ARE_ERRORS),y)
 CFLAGS += -Werror
 endif

Signed-off-by: Myles Watson

Thanks,
Myles

[Myles Watson]

On Fri, Mar 5, 2010 at 8:04 AM, Myles Watson  wrote:
> This patch removes most of the rest of the compilation warnings for me.
> Move run_bios prototype to device.h
> Use time.h for get_time()

2b. Move tb_freq into functions.c instead of the time.h
> Move read_io and write_io to io.c and make them static
> Make a couple of functions static in interrupt.c
> Refactor a cast from char[] to u64 to get rid of potential alignment
>
> problems and a warning
>
> The only ones left are "unused function" warnings.
>
> I think we should get rid of that warning, since we conditionally call
> functions based on debugging and various config variables.  Is there a case
> where it helps enough to justify all the warnings?
>
> This next part isn't part of the patch, but applying it makes qemu compile
> with yabel (with and without debugging).
> Index: Makefile
>
> --- Makefile    (revision 5186)
> +++ Makefile    (working copy)
> @@ -239,7 +239,7 @@
>  CFLAGS = $(INCLUDES) -Os -nostdinc
>  CFLAGS += -nostdlib -Wall -Wundef -Wstrict-prototypes -Wmissing-prototypes
>  CFLAGS += -Wwrite-strings -Wredundant-decls -Wno-trigraphs
> -CFLAGS += -Wstrict-aliasing -Wshadow

I meant no-unused-function:
> +CFLAGS += -Wstrict-aliasing -Wshadow -Wno-unused-function
> ifeq ($(CONFIG_WARNINGS_ARE_ERRORS),y)
>  CFLAGS += -Werror
>  endif

Signed-off-by: Myles Watson

Thanks,
Myles

[Stefan Reinauer]

On 3/5/10 4:13 PM, Myles Watson wrote:
> I think we should get rid of that warning, since we conditionally
> call functions based on debugging and various config variables.  

I think we should instead just drop CONFIG_WARNINGS_ARE_ERRORS for Qemu
rather than dropping warnings. Or maybe only add -Wno-unused-function if
CONFIG_WARNINGS_ARE_ERRORS is active, at least.  Or maybe add an Option
"Warnings good for Developers"?
> Is there a case where it helps enough to justify all the warnings?

Yes, they indicate that there is dead code. This is, not only but
especially useful to recognize if / how code should / could be restructured.
> Signed-off-by: Myles Watson <[e-mail]
> >

With the -Wno-unused-functions taken care of:

Acked-by: Stefan Reinauer

[Myles Watson]

On Fri, Mar 5, 2010 at 10:28 AM, Stefan Reinauer wrote:
> On 3/5/10 4:13 PM, Myles Watson wrote:
>
> I think we should get rid of that warning, since we conditionally call
> > functions based on debugging and various config variables.
>
> I think we should instead just drop CONFIG_WARNINGS_ARE_ERRORS for Qemu
> rather than dropping warnings.

I think that setting for Qemu has reduced the number of warnings in our code
by a lot.  It's easier for the original developer to deal with warnings when
the code is written.
> Or maybe only add -Wno-unused-function if

CONFIG_WARNINGS_ARE_ERRORS is active, at least.  Or maybe add an Option
> "Warnings good for Developers"?

I can see your point.  I'd rather not have to special case warnings either.
What would you suggest for the two emulator functions in yabel that are
unused right now?  Should we comment them out until they are used?
> Is there a case where it helps enough to justify all the warnings?
> >
>
> Yes, they indicate that there is dead code. This is, not only but
> especially useful to recognize if / how code should / could be restructured.

I worry that having too many warnings makes it so that "important" ones
(ones that cause bugs) get missed.
> Signed-off-by: Myles Watson
>
> With the -Wno-unused-functions taken care of:

I wasn't sure how you wanted it taken care of.  I left it for another patch.
> Acked-by: Stefan Reinauer 

Rev 5191.

Thanks,
Myles