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: Fri, 08 May 2009 14:34:09 +0200	[thread overview]
Message-ID: <m23abf4vum.fsf@ohwell.denx.de> (raw)
In-Reply-To: <200905071739.56301.sr@denx.de> (Stefan Roese's message of "Thu,  7 May 2009 17:39:56 +0200")

Hi Stefan,

>> > 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.
>
> BTW: The resulting code/data length is the same, comparing a version with 
> #ifdef's, the attribute version or a version with the variable declaration 
> intact and the warnings.

Actually my argument was never about resulting code or data size, but
about maintainability.

My clear view here is to put as much information as possible into the
sources.  So if at all possible, let the compiler know.  If this is not
possible then let the user know by using constructs that at least show a
connection to the human reader - i.e. using ifdef blocks with the same
name.  If that is also not possible then the least we should do is to
write a comment with the original intent.

>> 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.
>
> This could be the case. But this could happen to the #ifdef version as well. 
> That the #ifdef'ed variable declaration stays in the code after removing the 
> code referencing the variables.

Someone touching one block with ifdefs will surely - or better should
and *can* - check other blocks with the same condition.  Using an
__unused attribute breaks this connection.

Cheers
  Detlev

-- 
By all means let's be open-minded, but not so open-minded that our
brains drop out.
                                                -- Richard Dawkins
--
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

  parent reply	other threads:[~2009-05-08 12:34 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
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 [this message]
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=m23abf4vum.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