public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Detlev Zundel <dzu@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] ppc4xx: Add Sequoia RAM-booting target
Date: Thu, 07 May 2009 17:06:31 +0200	[thread overview]
Message-ID: <m2ocu56jgo.fsf@ohwell.denx.de> (raw)
In-Reply-To: <200905071530.43940.sr@denx.de> (Stefan Roese's message of "Thu,  7 May 2009 15:30:43 +0200")

Hi Stefan,

> Hi Detlev,
>
> On Thursday 07 May 2009, Detlev Zundel wrote:
>> >  int misc_init_r(void)
>> >  {
>> > -	uint pbcr;
>> > -	int size_val = 0;
>> > -	u32 reg;
>> > +	__attribute__((unused)) uint pbcr;
>> > +	__attribute__((unused)) int size_val = 0;
>> > +	__attribute__((unused)) u32 reg;
>>
>> Am I correct to assume that this should shut up warnings for the ifdef
>> case?
>
> Yes.
>
>> If so, it still seems to be a somewhat rude way to do it.  How
>> long will it take the gcc maintainers to produce a "warning: unused
>> variable is used" warning? ;)
>
> I prefer to do it this way instead of encasing the variable declaration into 
> another #ifdef ... #endif section. This is used in many cases in the Linux 
> kernel btw. Here the macro "__maybe_unsed" is defined to 
> "__attribute__((unused))".

In many cases?  a rgrep on a recent kernel counts 84 incantations, which
is not much for the Linux kernel, I believe.

> So what should I do now? Should I revert to another #ifdef in the variable 
> declaration? Or is the current version ok?

I'm not too sure myself.  What really tickles me, and what speaks
against using this attribute, is the fact that the "unused" attribute is
itself not part of an #ifdef, whereas the intention clearly is that this
attribute should only be applied when the ifdefs erases code.  

Now currently this connection maybe clear for the writer of the patch,
but it is in no way obvious in the code.  So theoretically, when the
#ifdef gets removed, nobody will think about the "unused" attributes,
forget them and then we have effectively lost correct warnings.

What do other people think?

Cheers
  Detlev

-- 
(let ((s "bottles of beer on the wall")) ((lambda (f) (f f 99))
(lambda (f i) (or (= i 0) (format #t "~a ~a - take one down pass it around
~a ~a\n" i s (- i 1) s) (f f (- i 1))))))
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

  reply	other threads:[~2009-05-07 15:06 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-05 15:01 [U-Boot] [PATCH] ppc4xx: Add Sequoia RAM-booting target Stefan Roese
2009-05-07 12:25 ` Detlev Zundel
2009-05-07 13:30   ` Stefan Roese
2009-05-07 15:06     ` Detlev Zundel [this message]
2009-05-07 15:39       ` Stefan Roese
2009-05-07 19:06         ` Wolfgang Denk
2009-05-07 19:22           ` Scott Wood
2009-05-08  4:30           ` Stefan Roese
2009-05-08 12:34         ` Detlev Zundel
2009-05-07 18:42     ` Wolfgang Denk

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=m2ocu56jgo.fsf@ohwell.denx.de \
    --to=dzu@denx.de \
    --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