config.mk: Add -Wundef to CFLAGS
PATCH 1/1 config.mk: Add -Wundef to CFLAGS
In U-boot source, some '#if' directives evaluate
undefined identifiers.
To find and fix them, this commit adds -Wundef to CFLAGS.
--- config.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
config.mk +1 -1
@@ -259,7 +259,7 @@ CPPFLAGS += -I$(TOPDIR)/include CPPFLAGS += -fno-builtin -ffreestanding -nostdinc \ -isystem $(gccincdir) -pipe $(PLATFORM_CPPFLAGS) -CFLAGS := $(CPPFLAGS) -Wall -Wstrict-prototypes +CFLAGS := $(CPPFLAGS) -Wall -Wstrict-prototypes -Wundef ifdef BUILD_TAG CFLAGS += -DBUILD_TAG='"$(BUILD_TAG)"'
Signature · 1 line
1.8.1.2
PATCH 1/1 config.mk: Add -Wundef to CFLAGS
Hi Masahiro,
Quoted 15 lines · Masahiro Yamada
In U-boot source, some '#if' directives evaluate
undefined identifiers.To find and fix them, this commit adds -Wundef to CFLAGS.
Signed-off-by Masahiro Yamada <[email protected]>--- config.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)config.mk +0
-0
config.mk CPPFLAGS += -I$(TOPDIR)/include · line 265
Quoted 8 lines
config.mk +1
-1@@ -259,7 +259,7 @@ CPPFLAGS += -I$(TOPDIR)/include CPPFLAGS += -fno-builtin -ffreestanding -nostdinc \ -isystem $(gccincdir) -pipe $(PLATFORM_CPPFLAGS) -CFLAGS := $(CPPFLAGS) -Wall -Wstrict-prototypes +CFLAGS := $(CPPFLAGS) -Wall -Wstrict-prototypes -Wundef ifdef BUILD_TAG CFLAGS += -DBUILD_TAG='"$(BUILD_TAG)"'
Will the patch cause some targets to break? If so, then I personally would prefer that fixes be provided for at least the trivial cases.
Amicalement,
Signature · 1 line
Albert.
PATCH 1/1 config.mk: Add -Wundef to CFLAGS
Hello Albert
Quoted 1 line
Will the patch cause some targets to break?
I don't think so
because -Wundef option just prints warnings.
GCC manual says:
-Wundef
Warn if an undefined identifier is evaluated in an ?#if? directive.
After applying this patch,
I noticed warnings like follows:
include/common.h:240:11: warning: "CONFIG_ENV_ADDR" is not defined [-Wundef] include/common.h:240:46: warning: "CONFIG_SYS_MONITOR_BASE" is not defined [-Wundef] include/common.h:241:3: warning: "CONFIG_ENV_ADDR" is not defined [-Wundef] include/common.h:241:23: warning: "CONFIG_SYS_MONITOR_BASE" is not defined [-Wundef] include/common.h:241:49: warning: "CONFIG_SYS_MONITOR_LEN" is not defined [-Wundef]
If this patch is applied soon, we have enough time to fix warnings before u-boot-2013.10 release.
If I have time, I'd like to make effort to eliminate such warnings. Of course, patches from other contributers are very welcome.
Best Regards
Masahiro Yamada
PATCH 1/1 config.mk: Add -Wundef to CFLAGS
Hi Masahiro,
Quoted 24 lines · Masahiro Yamada
Hello Albert
Quoted 1 line
Will the patch cause some targets to break?
I don't think so
because -Wundef option just prints warnings.GCC manual says:
-Wundef
Warn if an undefined identifier is evaluated in an ?#if? directive.After applying this patch,
I noticed warnings like follows:
include/common.h:240:11: warning: "CONFIG_ENV_ADDR" is not defined [-Wundef]include/common.h:240:46: warning: "CONFIG_SYS_MONITOR_BASE" is not defined [-Wundef]include/common.h:241:3: warning: "CONFIG_ENV_ADDR" is not defined [-Wundef]include/common.h:241:23: warning: "CONFIG_SYS_MONITOR_BASE" is not defined [-Wundef]include/common.h:241:49: warning: "CONFIG_SYS_MONITOR_LEN" is not defined [-Wundef]
Quoted 5 lines
If this patch is applied soon, we have enough time to fix warnings before u-boot-2013.10 release.
If I have time, I'd like to make effort to eliminate such warnings. Of course, patches from other contributers are very welcome.
I U-Boot we have a policy that "warnings are errors", so yes, this change breaks some boards.
I would indeed ask that any warnings this swtich causes to appear should be fixed.
However, the problem is you're doing a change across *all architectures* but will most probably only test it for a few, if not only one.
This means your patch will require *all* custodians to verify that everything keeps building fine for *their* architecture.
[ $ sudo wdenk || echo "Oh well." ]
Therefore I ask:
- that this patch be submitted along fixes to build failures it causes, as a proper patch series, by a single individual, or collected by someone in an officially created git repo or branch;
- that the series be tested before submission for at least one target of each architecture (not necessarily by a single person, though:
if collected in a repo or branch, testing would occur when people submit individual fixes, and the fix would only be accepted if such testing has been done);
- that whenever a version of the series is submitted, all architecture custodians be CC:ed and asked explicitly to build the series for the whole of their architecture (for ARM, you can Cc: me only, as I also build all the other ARM trees);
- that the custodians report to the submitter all build failures, and that the submitter fix and tests for any such report (or again, gets it tested by someone else)
[ exit ]
I'd love to see this warning in; but I'd hate to see uncoordinated, loosely inter-related, patches from various individuals ending up as noise. Let's make this in an organized fashion.
Quoted 2 lines
Best Regards
Masahiro Yamada
Amicalement,
Signature · 1 line
Albert.
PATCH 1/1 config.mk: Add -Wundef to CFLAGS
Hello, Albert and U-Boot developers.
The current status of this patch is Changes Requested.
I love -Wundef option to be in, but it looks like
difficult for me to post the version 2.
The first choice to meet Albert's requirement is
Quoted 4 lines
Therefore I ask:
- that this patch be submitted along fixes to build failures it causes, as a proper patch series, by a single individual,
Sorry, I cannot do this because:
I am not familiar with architectures other than ARM. I understand only a few devices.
To fix warnings in a correct way, a close look is often needed, but I cannot cover the whole code in the U-Boot tree.
If possible, could anyone take over this task?
The other option is
Quoted 1 line
collected by someone in an officially created git repo or branch;
OK, I can do this.
But I am not sure this will go well.
Even if I create a new repo u-boot-wundef, how many people will pay attention to this repository?
Most of users/developers track upstream repos
where -Wundef warnings are never displayed.
This means no one will have the motivation to fix the warnings.
If this patch is desired, in which way should we continue? Comments are welcome.
Best Regards,
Masahiro Yamada
PATCH 1/1 config.mk: Add -Wundef to CFLAGS
Hi Masahiro,
Quoted 40 lines · Masahiro Yamada
Hello, Albert and U-Boot developers.
The current status of this patch is Changes Requested.
I love -Wundef option to be in, but it looks like
difficult for me to post the version 2.The first choice to meet Albert's requirement is
Quoted 4 lines
Therefore I ask:
- that this patch be submitted along fixes to build failures it causes, as a proper patch series, by a single individual,
Sorry, I cannot do this because:
I am not familiar with architectures other than ARM. I understand only a few devices.
To fix warnings in a correct way, a close look is often needed, but I cannot cover the whole code in the U-Boot tree.If possible, could anyone take over this task?
The other option is
Quoted 1 line
collected by someone in an officially created git repo or branch;
OK, I can do this.
But I am not sure this will go well.Even if I create a new repo u-boot-wundef, how many people will pay attention to this repository?
Most of users/developers track upstream repos
where -Wundef warnings are never displayed.
Quoted 6 lines
This means no one will have the motivation to fix the warnings.
If this patch is desired, in which way should we continue? Comments are welcome.
Sorry not to have followed up earlier.
As I said, I want fixes for trivial cases -- cases where, for instance, a macro is used in an #if which has absolutely *no* definition in the whole codebase. I do not want fixes for all cases.
OTOH, to find out which failures would be trivial to fix and which ones would not, you'd have to go through all of them, which could be time-consuming, depending on the number of targets.
I thus suggest we use the typical U-Boot strategy: right at the beginning of the merge period, we apply the -Wundef patch onto all repos (or on the main repo and then pulled into others) and then wait for screams.
Screams should come fst from custodians, who routinely build for all targets of their assigned architecture. They will see which boards fail due to undefined macros and will report those failures on the list, copying the board maintainers (or subsystem owners if the issue is not board-specific.
All boards not fixed before merge window closure release will be declared dying; all those not fixed before 2014.01 will be declared dead, and orphaned and put in the scrapyard (and then, people who want them back can always resurrect *and fix* them).
Subsystems... will have to be fixed some way or other.
Of course, anyone with an interest can spontaneously provide fixes for boards or subsystems affected.
Adding Tom and Wolfgang for advice, but of course comments are welcome from everyone.
Quoted 2 lines
Best Regards,
Masahiro Yamada
Amicalement,
Signature · 1 line
Albert.
PATCH 1/1 config.mk: Add -Wundef to CFLAGS
Hi,
Quoted 78 lines · Albert ARIBAUD
Hi Masahiro,
Quoted 40 lines · Masahiro Yamada
Hello, Albert and U-Boot developers.
The current status of this patch is Changes Requested.
I love -Wundef option to be in, but it looks like
difficult for me to post the version 2.The first choice to meet Albert's requirement is
Quoted 4 lines
Therefore I ask:
- that this patch be submitted along fixes to build failures it causes, as a proper patch series, by a single individual,
Sorry, I cannot do this because:
I am not familiar with architectures other than ARM. I understand only a few devices.
To fix warnings in a correct way, a close look is often needed, but I cannot cover the whole code in the U-Boot tree.If possible, could anyone take over this task?
The other option is
Quoted 1 line
collected by someone in an officially created git repo or branch;
OK, I can do this.
But I am not sure this will go well.Even if I create a new repo u-boot-wundef, how many people will pay attention to this repository?
Most of users/developers track upstream repos
where -Wundef warnings are never displayed.Quoted 6 lines
This means no one will have the motivation to fix the warnings.
If this patch is desired, in which way should we continue? Comments are welcome.
Sorry not to have followed up earlier.
As I said, I want fixes for trivial cases -- cases where, for instance, a macro is used in an #if which has absolutely *no* definition in the whole codebase. I do not want fixes for all cases.
OTOH, to find out which failures would be trivial to fix and which ones would not, you'd have to go through all of them, which could be time-consuming, depending on the number of targets.
I thus suggest we use the typical U-Boot strategy: right at the beginning of the merge period, we apply the -Wundef patch onto all repos (or on the main repo and then pulled into others) and then wait for screams.
Screams should come fst from custodians, who routinely build for all targets of their assigned architecture. They will see which boards fail due to undefined macros and will report those failures on the list, copying the board maintainers (or subsystem owners if the issue is not board-specific.
All boards not fixed before merge window closure release will be declared dying; all those not fixed before 2014.01 will be declared dead, and orphaned and put in the scrapyard (and then, people who want them back can always resurrect *and fix* them).
Quoted 8 lines
Subsystems... will have to be fixed some way or other.
Of course, anyone with an interest can spontaneously provide fixes for boards or subsystems affected.
Adding Tom and Wolfgang for advice, but of course comments are welcome from everyone.
Breaking a board by introducing a warning is bad, I don't think we can do that.
But fixing common.h would be easy enough at least.
Can you run buildman on all boards and see how many files generate warnings?
Regards,
Simon
Quoted 12 lines
Quoted 3 lines
Best Regards,
Masahiro YamadaAmicalement,
--
Albert.
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
PATCH 1/1 config.mk: Add -Wundef to CFLAGS
Hi Simon,
Quoted 88 lines · Simon Glass
Hi,
Quoted 83 lines · Albert ARIBAUD
Hi Masahiro,
Quoted 40 lines · Masahiro Yamada
Hello, Albert and U-Boot developers.
The current status of this patch is Changes Requested.
I love -Wundef option to be in, but it looks like
difficult for me to post the version 2.The first choice to meet Albert's requirement is
Quoted 4 lines
Therefore I ask:
- that this patch be submitted along fixes to build failures it causes, as a proper patch series, by a single individual,
Sorry, I cannot do this because:
I am not familiar with architectures other than ARM. I understand only a few devices.
To fix warnings in a correct way, a close look is often needed, but I cannot cover the whole code in the U-Boot tree.If possible, could anyone take over this task?
The other option is
Quoted 1 line
collected by someone in an officially created git repo or branch;
OK, I can do this.
But I am not sure this will go well.Even if I create a new repo u-boot-wundef, how many people will pay attention to this repository?
Most of users/developers track upstream repos
where -Wundef warnings are never displayed.Quoted 6 lines
This means no one will have the motivation to fix the warnings.
If this patch is desired, in which way should we continue? Comments are welcome.
Sorry not to have followed up earlier.
As I said, I want fixes for trivial cases -- cases where, for instance, a macro is used in an #if which has absolutely *no* definition in the whole codebase. I do not want fixes for all cases.
OTOH, to find out which failures would be trivial to fix and which ones would not, you'd have to go through all of them, which could be time-consuming, depending on the number of targets.
I thus suggest we use the typical U-Boot strategy: right at the beginning of the merge period, we apply the -Wundef patch onto all repos (or on the main repo and then pulled into others) and then wait for screams.
Screams should come fst from custodians, who routinely build for all targets of their assigned architecture. They will see which boards fail due to undefined macros and will report those failures on the list, copying the board maintainers (or subsystem owners if the issue is not board-specific.
All boards not fixed before merge window closure release will be declared dying; all those not fixed before 2014.01 will be declared dead, and orphaned and put in the scrapyard (and then, people who want them back can always resurrect *and fix* them).
Subsystems... will have to be fixed some way or other.
Of course, anyone with an interest can spontaneously provide fixes for boards or subsystems affected.
Quoted 6 lines
Quoted 3 lines
Adding Tom and Wolfgang for advice, but of course comments are welcome from everyone.
Breaking a board by introducing a warning is bad, I don't think we can do that.
The idea *is* to break the board so that users or maintainers realize it builds on an undefined macro preprocessor symbol -- or so that we custodians realize no one is interested enough in that board to fix it, which is a sign that it should be removed from the code base. We've done such a thing before, actually, when a change required attention from a lot of maintainers.
Quoted 1 line
But fixing
common.hwould be easy enough at least.
That can be done for cases where the missing symbol should indeed exist. When it should not exist any more, nothing can fix it but removing the code that uses the zombie symbol. And in both cases, breaking the build with a warning is the best way to find and resolve the issue.
Quoted 1 line
Can you run buildman on all boards and see how many filesgenerate warnings?
I assume this request was directed at Masahiro Yamada.
I second the request: if only a few boards are affected, then instead of a system-wide change plus breaks, we could apply my first proposal, that is, Masahiro asking maintainers for changes and coordinating them in a branch, then a series.
Quoted 2 lines
Regards,
Simon
Amicalement,
Signature · 1 line
Albert.