dtc: Enable more warnings

dtc: Enable more warnings

[David Gibson]

This patch turns on a bunch of extra gcc warnings, most of which are
probably a good idea.  Of the new warnings -Wnested-externs and
-Wstrict-prototypes need no code changes, we're already warning-clean.
The remaining one, -Wmissing-prototypes requires trivial changes in
some of the tests (making functions local).

This patch also rearranges the warnings flags into a separate make
variable for convenience, and turns on -Werror, to really encourage
people to keep the code warning-clean.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>

Index: dtc/Makefile
===================================================================
--- dtc.orig/Makefile	2010-03-03 16:10:27.235986452 +1100
+++ dtc/Makefile	2010-03-03 16:22:17.439987248 +1100
@@ -16,7 +16,9 @@ LOCAL_VERSION =
 CONFIG_LOCALVERSION =
 
 CPPFLAGS = -I libfdt
-CFLAGS = -Wall -g -Os -fPIC -Wpointer-arith -Wcast-qual
+WARNINGS = -Werror -Wall  -Wpointer-arith -Wcast-qual -Wnested-externs \
+	-Wstrict-prototypes -Wmissing-prototypes
+CFLAGS = -g -Os -fPIC -Werror $(WARNINGS)
 
 CPPFLAGS += -std=c99 -D_XOPEN_SOURCE -D_BSD_SOURCE
 CFLAGS += -Werror
Index: dtc/tests/extra-terminating-null.c
===================================================================
--- dtc.orig/tests/extra-terminating-null.c	2010-03-03 16:16:22.635986164 +1100
+++ dtc/tests/extra-terminating-null.c	2010-03-03 16:16:40.535985904 +1100
@@ -28,7 +28,7 @@
 #include "tests.h"
 #include "testdata.h"
 
-void check_extranull(void *fdt, const char *prop, const char *str, int numnulls)
+static void check_extranull(void *fdt, const char *prop, const char *str, int numnulls)
 {
 	int len = strlen(str);
 	char checkbuf[len+numnulls];
Index: dtc/tests/get_alias.c
===================================================================
--- dtc.orig/tests/get_alias.c	2010-03-03 16:15:42.444985835 +1100
+++ dtc/tests/get_alias.c	2010-03-03 16:16:06.128012501 +1100
@@ -29,7 +29,7 @@
 #include "tests.h"
 #include "testdata.h"
 
-void check_alias(void *fdt, const char *path, const char *alias)
+static void check_alias(void *fdt, const char *path, const char *alias)
 {
 	const char *aliaspath;
 
Index: dtc/tests/path_offset_aliases.c
===================================================================
--- dtc.orig/tests/path_offset_aliases.c	2010-03-03 16:16:56.991986652 +1100
+++ dtc/tests/path_offset_aliases.c	2010-03-03 16:17:16.043985873 +1100
@@ -29,7 +29,7 @@
 #include "tests.h"
 #include "testdata.h"
 
-void check_alias(void *fdt, const char *full_path, const char *alias_path)
+static void check_alias(void *fdt, const char *full_path, const char *alias_path)
 {
 	int offset, offset_a;
 
Index: dtc/tests/value-labels.c
===================================================================
--- dtc.orig/tests/value-labels.c	2010-03-03 16:17:27.988986891 +1100
+++ dtc/tests/value-labels.c	2010-03-03 16:17:47.228985724 +1100
@@ -59,8 +59,8 @@ struct val_label labels3[] = {
 	{ "end3", -1 },
 };
 
-void check_prop_labels(void *sohandle, void *fdt, const char *name,
-		       const struct val_label* labels, int n)
+static void check_prop_labels(void *sohandle, void *fdt, const char *name,
+			      const struct val_label* labels, int n)
 {
 	const struct fdt_property *prop;
 	const char *p;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: dtc: Enable more warnings

[Jon Loeliger]

> This patch turns on a bunch of extra gcc warnings, most of which are
> probably a good idea.  Of the new warnings -Wnested-externs and
> -Wstrict-prototypes need no code changes, we're already warning-clean.
> The remaining one, -Wmissing-prototypes requires trivial changes in
> some of the tests (making functions local).
> 
> This patch also rearranges the warnings flags into a separate make
> variable for convenience, and turns on -Werror, to really encourage
> people to keep the code warning-clean.
> 
> Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>

This patch didn't apply due to something wrong here:


> Index: dtc/Makefile
> ===================================================================
> --- dtc.orig/Makefile	2010-03-03 16:10:27.235986452 +1100
> +++ dtc/Makefile	2010-03-03 16:22:17.439987248 +1100
> @@ -16,7 +16,9 @@ LOCAL_VERSION =
>  CONFIG_LOCALVERSION =
>  
>  CPPFLAGS = -I libfdt
> -CFLAGS = -Wall -g -Os -fPIC -Wpointer-arith -Wcast-qual
> +WARNINGS = -Werror -Wall  -Wpointer-arith -Wcast-qual -Wnested-externs \
> +	-Wstrict-prototypes -Wmissing-prototypes
> +CFLAGS = -g -Os -fPIC -Werror $(WARNINGS)
>  
>  CPPFLAGS += -std=c99 -D_XOPEN_SOURCE -D_BSD_SOURCE
>  CFLAGS += -Werror


I think it is the extra CPPFLAGS line?  I dont seem to have that.

Thanks,
jdl

Re: dtc: Enable more warnings

[David Gibson]

On Wed, Mar 03, 2010 at 08:16:16AM -0600, Jon Loeliger wrote:
> > This patch turns on a bunch of extra gcc warnings, most of which are
> > probably a good idea.  Of the new warnings -Wnested-externs and
> > -Wstrict-prototypes need no code changes, we're already warning-clean.
> > The remaining one, -Wmissing-prototypes requires trivial changes in
> > some of the tests (making functions local).
> > 
> > This patch also rearranges the warnings flags into a separate make
> > variable for convenience, and turns on -Werror, to really encourage
> > people to keep the code warning-clean.
> > 
> > Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> 
> This patch didn't apply due to something wrong here:
> 
> > Index: dtc/Makefile
> > ===================================================================
> > --- dtc.orig/Makefile	2010-03-03 16:10:27.235986452 +1100
> > +++ dtc/Makefile	2010-03-03 16:22:17.439987248 +1100
> > @@ -16,7 +16,9 @@ LOCAL_VERSION =
> >  CONFIG_LOCALVERSION =
> >  
> >  CPPFLAGS = -I libfdt
> > -CFLAGS = -Wall -g -Os -fPIC -Wpointer-arith -Wcast-qual
> > +WARNINGS = -Werror -Wall  -Wpointer-arith -Wcast-qual -Wnested-externs \
> > +	-Wstrict-prototypes -Wmissing-prototypes
> > +CFLAGS = -g -Os -fPIC -Werror $(WARNINGS)
> >  
> >  CPPFLAGS += -std=c99 -D_XOPEN_SOURCE -D_BSD_SOURCE
> >  CFLAGS += -Werror
> 
> 
> I think it is the extra CPPFLAGS line?  I dont seem to have that.

Drat, sorry.  I forgot I had an extra patch interposed in my series.
Resending.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

dtc: Enable more warnings

[David Gibson]

This patch turns on a bunch of extra gcc warnings, most of which are
probably a good idea.  Of the new warnings -Wnested-externs and
-Wstrict-prototypes need no code changes, we're already warning-clean.
The remaining one, -Wmissing-prototypes requires trivial changes in
some of the tests (making functions local).

This patch also rearranges the warnings flags into a separate make
variable for convenience, and turns on -Werror, to really encourage
people to keep the code warning-clean.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>

Index: dtc/Makefile
===================================================================
--- dtc.orig/Makefile	2010-03-04 11:09:54.527022044 +1100
+++ dtc/Makefile	2010-03-04 11:10:20.507033793 +1100
@@ -16,7 +16,9 @@ LOCAL_VERSION =
 CONFIG_LOCALVERSION =
 
 CPPFLAGS = -I libfdt
-CFLAGS = -Wall -g -Os -fPIC -Wpointer-arith -Wcast-qual
+WARNINGS = -Werror -Wall -Wextra -Wpointer-arith -Wcast-qual -Wnested-externs \
+	-Wstrict-prototypes -Wmissing-prototypes
+CFLAGS = -g -Os -fPIC -Werror $(WARNINGS)
 
 BISON = bison
 LEX = flex
Index: dtc/tests/extra-terminating-null.c
===================================================================
--- dtc.orig/tests/extra-terminating-null.c	2010-03-04 11:09:54.363021733 +1100
+++ dtc/tests/extra-terminating-null.c	2010-03-04 11:09:55.796048832 +1100
@@ -28,7 +28,7 @@
 #include "tests.h"
 #include "testdata.h"
 
-void check_extranull(void *fdt, const char *prop, const char *str, int numnulls)
+static void check_extranull(void *fdt, const char *prop, const char *str, int numnulls)
 {
 	int len = strlen(str);
 	char checkbuf[len+numnulls];
Index: dtc/tests/get_alias.c
===================================================================
--- dtc.orig/tests/get_alias.c	2010-03-04 11:09:54.335020966 +1100
+++ dtc/tests/get_alias.c	2010-03-04 11:09:55.796048832 +1100
@@ -29,7 +29,7 @@
 #include "tests.h"
 #include "testdata.h"
 
-void check_alias(void *fdt, const char *path, const char *alias)
+static void check_alias(void *fdt, const char *path, const char *alias)
 {
 	const char *aliaspath;
 
Index: dtc/tests/path_offset_aliases.c
===================================================================
--- dtc.orig/tests/path_offset_aliases.c	2010-03-04 11:09:54.379023209 +1100
+++ dtc/tests/path_offset_aliases.c	2010-03-04 11:09:55.796048832 +1100
@@ -29,7 +29,7 @@
 #include "tests.h"
 #include "testdata.h"
 
-void check_alias(void *fdt, const char *full_path, const char *alias_path)
+static void check_alias(void *fdt, const char *full_path, const char *alias_path)
 {
 	int offset, offset_a;
 
Index: dtc/tests/value-labels.c
===================================================================
--- dtc.orig/tests/value-labels.c	2010-03-04 11:09:54.355048932 +1100
+++ dtc/tests/value-labels.c	2010-03-04 11:09:55.796048832 +1100
@@ -59,8 +59,8 @@ struct val_label labels3[] = {
 	{ "end3", -1 },
 };
 
-void check_prop_labels(void *sohandle, void *fdt, const char *name,
-		       const struct val_label* labels, int n)
+static void check_prop_labels(void *sohandle, void *fdt, const char *name,
+			      const struct val_label* labels, int n)
 {
 	const struct fdt_property *prop;
 	const char *p;


-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: dtc: Enable more warnings

[Jon Loeliger]

> This patch turns on a bunch of extra gcc warnings, most of which are
> probably a good idea.  Of the new warnings -Wnested-externs and
> -Wstrict-prototypes need no code changes, we're already warning-clean.
> The remaining one, -Wmissing-prototypes requires trivial changes in
> some of the tests (making functions local).
> 
> This patch also rearranges the warnings flags into a separate make
> variable for convenience, and turns on -Werror, to really encourage
> people to keep the code warning-clean.
> 
> Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>

Well, I'd like to apply this patch.  Doing so is actually effective!

$ make check 2>&1 | less
        CHK version_gen.h
         CC tests/testutils.o
cc1: warnings being treated as errors
tests/testutils.c: In sigint_:
tests/testutils.c:44: error: unused 
tests/testutils.c:44: error: unused 
tests/testutils.c: In test_:
tests/testutils.c:52: error: unused 
tests/testutils.c: In check_:
tests/testutils.c:113: error: comparison between signed and unsigned
tests/testutils.c:117: error: comparison between signed and unsigned
make: *** [tests/testutils.o] Error 1
propertyfunction argcparameter initfunction ucparameter siparameter handlerfunction 

Re: dtc: Enable more warnings

[Jon Loeliger]

[-- Attachment #1: Type: text/plain, Size: 1363 bytes --]

> This patch turns on a bunch of extra gcc warnings, most of which are
> probably a good idea.  Of the new warnings -Wnested-externs and
> -Wstrict-prototypes need no code changes, we're already warning-clean.
> The remaining one, -Wmissing-prototypes requires trivial changes in
> some of the tests (making functions local).
> 
> This patch also rearranges the warnings flags into a separate make
> variable for convenience, and turns on -Werror, to really encourage
> people to keep the code warning-clean.
> 
> Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>

Well, I'd like to apply this patch.  But doing so appears
to be very effective.  So much so that the "make check"
now fails:

$ gcc --version
gcc (Debian 4.3.2-1.1) 4.3.2

$ make check
	CHK version_gen.h
	 CC tests/testutils.o
cc1: warnings being treated as errors
tests/testutils.c: In function ‘sigint_handler’:
tests/testutils.c:44: error: unused parameter ‘si’
tests/testutils.c:44: error: unused parameter ‘uc’
tests/testutils.c: In function ‘test_init’:
tests/testutils.c:52: error: unused parameter ‘argc’
tests/testutils.c: In function ‘check_property’:
tests/testutils.c:113: error: comparison between signed and unsigned
tests/testutils.c:117: error: comparison between signed and unsigned
make: *** [tests/testutils.o] Error 1

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: dtc: Enable more warnings

[David Gibson]

On Fri, Mar 05, 2010 at 03:46:38PM -0600, Jon Loeliger wrote:
> > This patch turns on a bunch of extra gcc warnings, most of which are
> > probably a good idea.  Of the new warnings -Wnested-externs and
> > -Wstrict-prototypes need no code changes, we're already warning-clean.
> > The remaining one, -Wmissing-prototypes requires trivial changes in
> > some of the tests (making functions local).
> > 
> > This patch also rearranges the warnings flags into a separate make
> > variable for convenience, and turns on -Werror, to really encourage
> > people to keep the code warning-clean.
> > 
> > Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> 
> Well, I'd like to apply this patch.  But doing so appears
> to be very effective.  So much so that the "make check"
> now fails:

Ugh.  That's the -Wextra.  Somehow I managed to add the -Wextra and
thought (to my surprise) that it didn't add any extra warnings over
the ones we already had.  I was wrong, obviously.  I'm not sure quite
how I managed to miss that.  I'll resend.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

dtc: Enable more warnings

[David Gibson]

This patch turns on a bunch of extra gcc warnings, most of which are
probably a good idea.  Of the new warnings -Wnested-externs and
-Wstrict-prototypes need no code changes, we're already warning-clean.
The remaining one, -Wmissing-prototypes requires trivial changes in
some of the tests (making functions local).

This patch also rearranges the warnings flags into a separate make
variable for convenience, and turns on -Werror, to really encourage
people to keep the code warning-clean.

Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>

---

Third time lucky.  Please?

Index: dtc/Makefile
===================================================================
--- dtc.orig/Makefile	2010-03-04 11:09:54.527022044 +1100
+++ dtc/Makefile	2010-03-09 10:48:23.343113245 +1100
@@ -16,7 +16,9 @@ LOCAL_VERSION =
 CONFIG_LOCALVERSION =
 
 CPPFLAGS = -I libfdt
-CFLAGS = -Wall -g -Os -fPIC -Wpointer-arith -Wcast-qual
+WARNINGS = -Werror -Wall -Wpointer-arith -Wcast-qual -Wnested-externs \
+	-Wstrict-prototypes -Wmissing-prototypes
+CFLAGS = -g -Os -fPIC -Werror $(WARNINGS)
 
 BISON = bison
 LEX = flex
Index: dtc/tests/extra-terminating-null.c
===================================================================
--- dtc.orig/tests/extra-terminating-null.c	2010-03-04 11:09:54.363021733 +1100
+++ dtc/tests/extra-terminating-null.c	2010-03-04 11:09:55.796048832 +1100
@@ -28,7 +28,7 @@
 #include "tests.h"
 #include "testdata.h"
 
-void check_extranull(void *fdt, const char *prop, const char *str, int numnulls)
+static void check_extranull(void *fdt, const char *prop, const char *str, int numnulls)
 {
 	int len = strlen(str);
 	char checkbuf[len+numnulls];
Index: dtc/tests/get_alias.c
===================================================================
--- dtc.orig/tests/get_alias.c	2010-03-04 11:09:54.335020966 +1100
+++ dtc/tests/get_alias.c	2010-03-04 11:09:55.796048832 +1100
@@ -29,7 +29,7 @@
 #include "tests.h"
 #include "testdata.h"
 
-void check_alias(void *fdt, const char *path, const char *alias)
+static void check_alias(void *fdt, const char *path, const char *alias)
 {
 	const char *aliaspath;
 
Index: dtc/tests/path_offset_aliases.c
===================================================================
--- dtc.orig/tests/path_offset_aliases.c	2010-03-04 11:09:54.379023209 +1100
+++ dtc/tests/path_offset_aliases.c	2010-03-04 11:09:55.796048832 +1100
@@ -29,7 +29,7 @@
 #include "tests.h"
 #include "testdata.h"
 
-void check_alias(void *fdt, const char *full_path, const char *alias_path)
+static void check_alias(void *fdt, const char *full_path, const char *alias_path)
 {
 	int offset, offset_a;
 
Index: dtc/tests/value-labels.c
===================================================================
--- dtc.orig/tests/value-labels.c	2010-03-04 11:09:54.355048932 +1100
+++ dtc/tests/value-labels.c	2010-03-04 11:09:55.796048832 +1100
@@ -59,8 +59,8 @@ struct val_label labels3[] = {
 	{ "end3", -1 },
 };
 
-void check_prop_labels(void *sohandle, void *fdt, const char *name,
-		       const struct val_label* labels, int n)
+static void check_prop_labels(void *sohandle, void *fdt, const char *name,
+			      const struct val_label* labels, int n)
 {
 	const struct fdt_property *prop;
 	const char *p;


-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: dtc: Enable more warnings

[Jon Loeliger]

> This patch turns on a bunch of extra gcc warnings, most of which are
> probably a good idea.  Of the new warnings -Wnested-externs and
> -Wstrict-prototypes need no code changes, we're already warning-clean.
> The remaining one, -Wmissing-prototypes requires trivial changes in
> some of the tests (making functions local).
> 
> This patch also rearranges the warnings flags into a separate make
> variable for convenience, and turns on -Werror, to really encourage
> people to keep the code warning-clean.
> 
> Signed-off-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> 
> ---
> 
> Third time lucky.  Please?

Applied!

jdl