linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: Daniel Santos <daniel.santos@pobox.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Andi Kleen <ak@linux.intel.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Christopher Li <sparse@chrisli.org>,
	David Daney <david.daney@cavium.com>,
	David Howells <dhowells@redhat.com>,
	Joe Perches <joe@perches.com>,
	Josh Triplett <josh@joshtriplett.org>,
	Konstantin Khlebnikov <khlebnikov@openvz.org>,
	linux-sparse@vger.kernel.org,
	Michel Lespinasse <walken@google.com>,
	Paul Gortmaker <paul.gortmaker@windriver.com>,
	Pavel Pisa <pisa@cmp.felk.cvut.cz>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Steven Rostedt <rostedt@goodmis.org>,
	David Rientjes <rientjes@google.com>
Subject: Re: [PATCH v4 6/9] compiler.h, bug.h: Prevent double error messages with BUILD_BUG{,_ON}
Date: Wed, 31 Oct 2012 12:06:10 +0100	[thread overview]
Message-ID: <20121031110609.GD16410@liondog.tnic> (raw)
In-Reply-To: <5090B875.7030902@att.net>

On Wed, Oct 31, 2012 at 12:34:45AM -0500, Daniel Santos wrote:
> Yes, the __build_bug_on_failed message is much more informative.  This
> will only increase with these patches.  For example, the line
> 
> BUILD_BUG_ON(sizeof(*c) != 4);
> 
> emits this error:
> 
> arch/x86/kernel/cpu/amd.c: In function ‘early_init_amd’:
> arch/x86/kernel/cpu/amd.c:486:2: error: call to
> ‘__build_bug_on_failed_486’ declared with attribute error: BUILD_BUG_ON
> failed: sizeof(*c) != 4
> make[1]: *** [arch/x86/kernel/cpu/amd.o] Error 1
> make: *** [arch/x86/kernel/cpu/amd.o] Error 2
> 
> It's true that there is some redundancy in there as well as the
> gibberish line number embedded in the function name, but the end of the
> line spits out the exact statement that failed.

I guess that's as good as it gets. But it's fine IMO, it tells you
exactly what you need to know.

> But as far as rather the fallback is first or the __compiletime_error
> function is a matter of asthetics, since it's really an either/or
> situation.  Either the __build_bug_on_failedxxx function will be
> declared with __attribute__((error(message))) and the fallback will
> expand to a no-op, or the fallback will produce code that (presumably
> always?) breaks the build.  For insurance, a link-time error will occur
> if the fallback code fails to break the build.

Right, but my suggestion was to have the more informative message always
trigger first, if possible and if gcc supports it (practically, more
and more systems will be upgrading gcc which has the error attribute
with time) and have the less informative one be the more seldom one. The
"fallback" naming is just a minor issue.

This way, the error message would be precise on most modern toolchains.
Older toolchains will issue something about negative array size, which
is not really helpful so one would have to fire up an editor and
actually look at the code :).

> Realistically, a single macro could be defined in compiler*.h that
> encapsulates the entirety of this mechanism and only exposes a "black
> box" macro, that will simply expand to something that breaks the build
> in the most appropriate fashion based upon the version of gcc.  In
> essence, the new BUILD_BUG_ON_MSG macro attempts to fill that roll.

Yes.

> I guess I'll fix it up (and address the emails on the other patches)
> and do a v5 then for the whole set? (is that the right way to resubmit
> with these corrections?)

Well, you could wait a couple of days first to gather feedback from
other people and then resend. This way you give chance to people to take
a look without them seeing too many versions of the patchset and getting
confused.

What I always do is send out the patchset, collect and discuss changes,
add in the required changes and test it while the discussions go on.
After they settle down (and they do in a couple of days, in most cases)
I then send out the newly tested version out.

That whole exercise takes more or less a week if you're doing other
stuff in between :)

-- 
Regards/Gruss,
    Boris.

  reply	other threads:[~2012-10-31 11:06 UTC|newest]

Thread overview: 187+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-28 23:20 [PATCH 0/10] Cleanup & new features for compiler*.h and bug.h Daniel Santos
2012-09-28 23:20 ` [PATCH 1/10] compiler-gcc4.h: correct verion check for __compiletime_error Daniel Santos
2012-10-03  6:25   ` David Rientjes
2012-10-11 20:54     ` Michal Marek
2012-09-28 23:20 ` [PATCH 2/10] compiler-gcc4.h: Reorder macros based upon gcc ver Daniel Santos
2012-10-03  6:28   ` David Rientjes
2012-09-28 23:20 ` [PATCH 3/10] compiler-gcc.h: Add gcc-recommended GCC_VERSION macro Daniel Santos
2012-09-30 13:20   ` Borislav Petkov
2012-09-30 23:11   ` Daniel Santos
2012-10-01  0:22     ` Josh Triplett
2012-10-03  6:32   ` David Rientjes
2012-09-28 23:20 ` [PATCH 4/10] compiler-gcc{3,4}.h: Use " Daniel Santos
2012-09-29  0:20   ` Josh Triplett
2012-09-29  0:31     ` [Bulk] " Daniel Santos
2012-09-29  0:42       ` Josh Triplett
2012-10-03  6:40         ` David Rientjes
2012-09-28 23:20 ` [PATCH 5/10] compiler{,-gcc4}.h: Remove duplicate macros Daniel Santos
2012-09-29  0:23   ` Josh Triplett
2012-09-29  0:34     ` [Bulk] " Daniel Santos
2012-09-29  0:48       ` Josh Triplett
2012-09-28 23:20 ` [PATCH 6/10] bug.h: Replace __linktime_error with __compiletime_error Daniel Santos
2012-09-29  0:23   ` Josh Triplett
2012-09-29  1:04     ` Steven Rostedt
2012-09-30 13:22       ` Borislav Petkov
2012-09-30 21:13         ` Daniel Santos
2012-10-03  6:44   ` David Rientjes
2012-10-03 11:49     ` Daniel Santos
2012-10-03 15:35       ` Josh Triplett
2012-10-03 18:26       ` David Rientjes
2012-10-04  0:26         ` Daniel Santos
2012-10-04 21:51           ` David Rientjes
2012-09-28 23:20 ` [PATCH 7/10] compiler{,-gcc4}.h: Introduce __flatten function attribute Daniel Santos
2012-09-29  0:26   ` Josh Triplett
2012-09-29  0:38     ` Daniel Santos
2012-09-29  0:50       ` Josh Triplett
2012-10-03  6:49         ` David Rientjes
2012-10-03  6:59           ` Josh Triplett
2012-10-03  7:53             ` David Rientjes
2012-10-03 11:20               ` Daniel Santos
2012-10-03 14:01                 ` Steven Rostedt
2012-10-03 14:46                   ` Daniel Santos
2012-10-03 15:14                     ` Steven Rostedt
2012-10-03 15:23                       ` Peter Zijlstra
2012-10-03 15:38                       ` Joe Perches
2012-10-04  0:32                         ` Steven Rostedt
2012-10-04  0:54                           ` Daniel Santos
2012-10-04  2:33                           ` Joe Perches
2012-10-04  0:55                     ` Michel Lespinasse
2012-09-28 23:20 ` [PATCH 8/10] bug.h: Make BUILD_BUG_ON generate compile-time error Daniel Santos
2012-09-29  0:32   ` Josh Triplett
2012-09-29  0:51     ` Daniel Santos
2012-09-29  1:26     ` Daniel Santos
2012-09-29  2:55       ` Josh Triplett
2012-09-30 23:29         ` Daniel Santos
2012-10-01  0:26           ` Josh Triplett
2012-10-02  0:48   ` Michel Lespinasse
2012-10-02 14:57     ` Daniel Santos
2012-09-28 23:20 ` [PATCH 9/10] bug.h: Add BUILD_BUG_ON_NON_CONST macro Daniel Santos
2012-09-28 23:20 ` [PATCH 10/10] bug.h: Add gcc 4.2+ versions of BUILD_BUG_ON_* macros Daniel Santos
2012-10-02  0:55   ` Michel Lespinasse
2012-10-02 16:04     ` Daniel Santos
2012-10-05 19:35 ` [PATCH v2 0/10] Cleanup & new features for compiler*.h and bug.h danielfsantos
2012-10-05 19:42   ` [PATCH v2 01/10] compiler-gcc4.h: Reorder macros based upon gcc ver danielfsantos
2012-10-06 17:42     ` Borislav Petkov
2012-10-06 17:54       ` Daniel Santos
2012-10-18  2:26         ` David Rientjes
2012-10-05 19:42   ` [PATCH v2 02/10] compiler-gcc.h: Add gcc-recommended GCC_VERSION macro danielfsantos
2012-10-05 19:42   ` [PATCH v2 03/10] compiler-gcc{3,4}.h: Use " danielfsantos
2012-10-06 23:05     ` Borislav Petkov
2012-10-06 23:10     ` Borislav Petkov
2012-10-07 18:27       ` Daniel Santos
2012-10-07 19:42         ` Borislav Petkov
2012-10-07 20:21           ` Daniel Santos
2012-10-09 18:41         ` Andrew Morton
2012-10-09 19:45           ` Josh Triplett
2012-10-05 19:42   ` [PATCH v2 04/10] bug.h: directly include linux/compiler.h danielfsantos
2012-10-05 19:42   ` [PATCH v2 05/10] compiler{,-gcc4}.h, bug.h: Remove duplicate macros danielfsantos
2012-10-05 19:42   ` [PATCH v2 06/10] bug.h: Make BUILD_BUG_ON generate compile-time error danielfsantos
2012-10-05 19:42   ` [PATCH v2 07/10] compiler.h, bug.h: Prevent double error messages with BUILD_BUG{,_ON} danielfsantos
2012-10-05 20:59     ` Josh Triplett
2012-10-06  4:28       ` Daniel Santos
2012-10-05 19:42   ` [PATCH v2 08/10] bug.h: Fix BUILD_BUG_ON macro in __CHECKER__ danielfsantos
2012-10-05 19:42   ` [PATCH v2 09/10] bug.h: Add BUILD_BUG_ON_MSG & BUILD_BUG_INTERNAL{,2} danielfsantos
2012-10-05 20:58     ` Borislav Petkov
2012-10-05 21:02       ` Josh Triplett
2012-10-06  4:41         ` Daniel Santos
2012-10-05 21:04       ` Steven Rostedt
2012-10-05 19:42   ` [PATCH v2 10/10] bug.h: Convert BUILD_BUG{,_ON} to use BUILD_BUG_ON_MSG danielfsantos
2012-10-05 20:27   ` [PATCH v2 0/10] Cleanup & new features for compiler*.h and bug.h Steven Rostedt
2012-10-07 18:36   ` Daniel Santos
2012-10-24 16:28 ` [PATCH v3 " danielfsantos
2012-10-24 16:33   ` [PATCH v3 01/10] compiler-gcc4.h: Reorder macros based upon gcc ver danielfsantos
2012-10-24 16:33   ` [PATCH v3 02/10] compiler-gcc.h: Add gcc-recommended GCC_VERSION macro danielfsantos
2012-10-24 16:33   ` [PATCH v3 03/10] compiler-gcc{3,4}.h: Use " danielfsantos
2012-10-24 19:05     ` Borislav Petkov
2012-10-24 21:49       ` Josh Triplett
2012-10-24 22:34         ` Borislav Petkov
2012-10-24 16:33   ` [PATCH v3 04/10] bug.h: directly include linux/compiler.h danielfsantos
2012-10-24 19:55     ` Borislav Petkov
2012-10-28 19:23       ` Daniel Santos
2012-10-24 16:33   ` [PATCH v3 05/10] compiler{,-gcc4}.h, bug.h: Remove duplicate macros danielfsantos
2012-10-25  9:26     ` Borislav Petkov
2012-10-24 16:33   ` [PATCH v3 06/10] bug.h: Make BUILD_BUG_ON generate compile-time error danielfsantos
2012-10-25  9:33     ` Borislav Petkov
2012-10-24 16:33   ` [PATCH v3 07/10] compiler.h, bug.h: Prevent double error messages with BUILD_BUG{,_ON} danielfsantos
2012-10-24 16:33   ` [PATCH v3 08/10] bug.h: Fix BUILD_BUG_ON macro in __CHECKER__ danielfsantos
2012-10-24 16:34   ` [PATCH v3 09/10] bug.h: Add BUILD_BUG_ON_MSG & _BUILD_BUG_INTERNAL danielfsantos
2012-10-24 16:34   ` [PATCH v3 10/10] bug.h: Convert BUILD_BUG{,_ON} to use BUILD_BUG_ON_MSG danielfsantos
2012-10-28 20:53 ` [PATCH v4 0/10] Cleanup & new features for compiler*.h and bug.h danielfsantos
2012-10-28 20:57   ` [PATCH v4 1/9] compiler-gcc4.h: Reorder macros based upon gcc ver danielfsantos
2012-10-30 12:02     ` Borislav Petkov
2012-10-28 20:57   ` [PATCH v4 2/9] compiler-gcc.h: Add gcc-recommended GCC_VERSION macro danielfsantos
2012-10-28 20:57   ` [PATCH v4 3/9] compiler-gcc{3,4}.h: Use " danielfsantos
2012-10-28 20:57   ` [PATCH v4 4/9] compiler{,-gcc4}.h, bug.h: Remove duplicate macros danielfsantos
2012-10-28 20:57   ` [PATCH v4 5/9] bug.h: Make BUILD_BUG_ON generate compile-time error danielfsantos
2012-10-28 20:57   ` [PATCH v4 6/9] compiler.h, bug.h: Prevent double error messages with BUILD_BUG{,_ON} danielfsantos
2012-10-30 16:19     ` Borislav Petkov
2012-10-31  5:34       ` Daniel Santos
2012-10-31 11:06         ` Borislav Petkov [this message]
2012-10-31 16:38           ` Daniel Santos
2012-11-03 18:10           ` Daniel Santos
2012-10-28 20:57   ` [PATCH v4 7/9] bug.h: Fix BUILD_BUG_ON macro in __CHECKER__ danielfsantos
2012-10-30 16:44     ` Borislav Petkov
2012-10-28 20:57   ` [PATCH v4 8/9] bug.h: Add BUILD_BUG_ON_MSG & _BUILD_BUG_INTERNAL danielfsantos
2012-10-30 17:17     ` Borislav Petkov
2012-10-30 21:57       ` Borislav Petkov
2012-10-28 20:57   ` [PATCH v4 9/9] bug.h: Convert BUILD_BUG{,_ON} to use BUILD_BUG_ON_MSG danielfsantos
2012-10-30 19:19     ` Borislav Petkov
2012-10-31  1:02       ` Josh Triplett
2012-10-31  5:48         ` Daniel Santos
2012-11-13 22:09 ` [PATCH v5 0/9] Cleanup & new features for compiler*.h and bug.h danielfsantos
2012-11-13 22:13   ` [PATCH v5 1/9] compiler-gcc4.h: Reorder macros based upon gcc ver danielfsantos
2012-11-13 22:13   ` [PATCH v5 2/9] compiler-gcc.h: Add gcc-recommended GCC_VERSION macro danielfsantos
2012-11-13 22:13   ` [PATCH v5 3/9] compiler-gcc{3,4}.h: Use " danielfsantos
2012-11-13 22:13   ` [PATCH v5 4/9] compiler{,-gcc4}.h, bug.h: Remove duplicate macros danielfsantos
2012-11-13 22:13   ` [PATCH v5 5/9] bug.h: Fix BUILD_BUG_ON macro in __CHECKER__ danielfsantos
2012-11-15 15:07     ` Borislav Petkov
2012-11-13 22:13   ` [PATCH v5 6/9] bug.h: Prevent double evaulation of in BUILD_BUG_ON danielfsantos
2012-11-15 15:07     ` Borislav Petkov
2012-11-15 19:11       ` Daniel Santos
2012-11-17 14:38         ` Borislav Petkov
2012-11-20 19:10           ` Daniel Santos
2012-11-13 22:13   ` [PATCH v5 7/9] bug.h: Make BUILD_BUG_ON generate compile-time error danielfsantos
2012-11-13 22:24     ` Daniel Santos
2012-11-15 15:07       ` Borislav Petkov
2012-11-13 22:13   ` [PATCH v5 8/9] compiler.h, bug.h: Prevent double error messages with BUILD_BUG{,_ON} danielfsantos
2012-11-15 15:08     ` Borislav Petkov
2012-11-15 19:44       ` Daniel Santos
2012-11-17 14:39         ` Borislav Petkov
2012-11-13 22:13   ` [PATCH v5 9/9] bug.h, compiler.h: Introduce compiletime_assert & BUILD_BUG_ON_MSG danielfsantos
2012-11-15 15:08     ` Borislav Petkov
2012-11-16 23:25     ` Daniel Santos
2012-11-20 20:42 ` [PATCH v6 0/9] Cleanup & new features for compiler*.h and bug.h danielfsantos
2012-11-20 21:04   ` [PATCH v6 1/9] compiler-gcc4.h: Reorder macros based upon gcc ver danielfsantos
2012-11-20 21:05   ` [PATCH v6 2/9] compiler-gcc.h: Add gcc-recommended GCC_VERSION macro danielfsantos
2012-11-20 21:05   ` [PATCH v6 3/9] compiler-gcc{3,4}.h: Use " danielfsantos
2012-11-20 21:05   ` [PATCH v6 4/9] compiler{,-gcc4}.h, bug.h: Remove duplicate macros danielfsantos
2012-11-20 21:05   ` [PATCH v6 5/9] bug.h: Fix BUILD_BUG_ON macro in __CHECKER__ danielfsantos
2012-11-20 21:05   ` [PATCH v6 6/9] bug.h: Prevent double evaulation of in BUILD_BUG_ON danielfsantos
2012-11-22 13:42     ` Borislav Petkov
2012-11-20 21:05   ` [PATCH v6 7/9] bug.h: Make BUILD_BUG_ON generate compile-time error danielfsantos
2012-11-20 21:05   ` [PATCH v6 8/9] compiler.h, bug.h: Prevent double error messages with BUILD_BUG{,_ON} danielfsantos
2012-11-22 13:43     ` Borislav Petkov
2012-11-20 21:05   ` [PATCH v6 9/9] bug.h, compiler.h: Introduce compiletime_assert & BUILD_BUG_ON_MSG danielfsantos
2012-11-21  2:35   ` [PATCH v6 0/9] Cleanup & new features for compiler*.h and bug.h Michel Lespinasse
2012-11-22 13:44   ` Borislav Petkov
2013-01-01 21:08 ` [PATCH v7 " danielfsantos
2013-01-01 21:09   ` [PATCH v7 1/9] compiler-gcc4.h: Reorder macros based upon gcc ver danielfsantos
2013-01-01 21:09     ` [PATCH v7 2/9] compiler-gcc.h: Add gcc-recommended GCC_VERSION macro danielfsantos
2013-01-01 21:09     ` [PATCH v7 3/9] compiler-gcc{3,4}.h: Use " danielfsantos
2013-01-01 21:09     ` [PATCH v7 4/9] compiler{,-gcc4}.h, bug.h: Remove duplicate macros danielfsantos
2013-01-01 21:09     ` [PATCH v7 5/9] bug.h: Fix BUILD_BUG_ON macro in __CHECKER__ danielfsantos
2013-01-01 21:09     ` [PATCH v7 6/9] bug.h: Prevent double evaulation of in BUILD_BUG_ON danielfsantos
2013-01-01 21:09     ` [PATCH v7 7/9] bug.h: Make BUILD_BUG_ON generate compile-time error danielfsantos
2013-01-01 21:09     ` [PATCH v7 8/9] compiler.h, bug.h: Prevent double error messages with BUILD_BUG{,_ON} danielfsantos
2013-01-01 21:09     ` [PATCH v7 9/9] bug.h, compiler.h: Introduce compiletime_assert & BUILD_BUG_ON_MSG danielfsantos
2013-01-01 21:27     ` [PATCH v7 1/9] compiler-gcc4.h: Reorder macros based upon gcc ver Daniel Santos
2013-01-01 22:54 ` [PATCH v8 0/9] Cleanup & new features for compiler*.h and bug.h danielfsantos
2013-01-01 22:54   ` [PATCH v8 1/9] compiler-gcc4.h: Reorder macros based upon gcc ver danielfsantos
2013-01-01 22:54   ` [PATCH v8 2/9] compiler-gcc.h: Add gcc-recommended GCC_VERSION macro danielfsantos
2013-01-01 22:54   ` [PATCH v8 3/9] compiler-gcc{3,4}.h: Use " danielfsantos
2013-01-01 22:54   ` [PATCH v8 4/9] compiler{,-gcc4}.h, bug.h: Remove duplicate macros danielfsantos
2013-01-01 22:54   ` [PATCH v8 5/9] bug.h: Fix BUILD_BUG_ON macro in __CHECKER__ danielfsantos
2013-01-01 22:54   ` [PATCH v8 6/9] bug.h: Prevent double evaulation of in BUILD_BUG_ON danielfsantos
2013-01-01 22:54   ` [PATCH v8 7/9] bug.h: Make BUILD_BUG_ON generate compile-time error danielfsantos
2013-01-01 22:54   ` [PATCH v8 8/9] compiler.h, bug.h: Prevent double error messages with BUILD_BUG{,_ON} danielfsantos
2013-01-01 22:54   ` [PATCH v8 9/9] bug.h, compiler.h: Introduce compiletime_assert & BUILD_BUG_ON_MSG danielfsantos

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=20121031110609.GD16410@liondog.tnic \
    --to=bp@alien8.de \
    --cc=a.p.zijlstra@chello.nl \
    --cc=aarcange@redhat.com \
    --cc=ak@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=daniel.santos@pobox.com \
    --cc=david.daney@cavium.com \
    --cc=dhowells@redhat.com \
    --cc=joe@perches.com \
    --cc=josh@joshtriplett.org \
    --cc=khlebnikov@openvz.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sparse@vger.kernel.org \
    --cc=paul.gortmaker@windriver.com \
    --cc=pisa@cmp.felk.cvut.cz \
    --cc=rientjes@google.com \
    --cc=rostedt@goodmis.org \
    --cc=sparse@chrisli.org \
    --cc=walken@google.com \
    /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;
as well as URLs for NNTP newsgroup(s).