All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.