public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Albert ARIBAUD <albert.u.boot@aribaud.net>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] config.mk: Add -Wundef to CFLAGS
Date: Wed, 31 Jul 2013 09:26:53 +0200	[thread overview]
Message-ID: <20130731092653.16a05391@lilith> (raw)
In-Reply-To: <20130731114517.AA41.AA925319@jp.panasonic.com>

Hi Masahiro,

On Wed, 31 Jul 2013 11:45:18 +0900, Masahiro Yamada
<yamada.m@jp.panasonic.com> wrote:

> Hello Albert
> 
> > 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.

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.

> Best Regards
> Masahiro Yamada

Amicalement,
-- 
Albert.

  reply	other threads:[~2013-07-31  7:26 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-30  2:57 [U-Boot] [PATCH] config.mk: Add -Wundef to CFLAGS Masahiro Yamada
2013-07-30 12:07 ` Albert ARIBAUD
2013-07-31  2:45   ` Masahiro Yamada
2013-07-31  7:26     ` Albert ARIBAUD [this message]
2013-08-21  4:33       ` Masahiro Yamada
2013-10-02 19:27         ` Albert ARIBAUD
2013-10-03 23:49           ` Simon Glass
2013-10-05  7:04             ` Albert ARIBAUD

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130731092653.16a05391@lilith \
    --to=albert.u.boot@aribaud.net \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox