All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boaz Harrosh <bharrosh@panasas.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Rusty Russell <rusty@rustcorp.com.au>,
	Alexey Dobriyan <adobriyan@gmail.com>,
	torvalds@linuxfoundation.org, akpm@linuxfoundation.org,
	linux-kernel@vger.kernel.org, Sam Ravnborg <sam@ravnborg.org>
Subject: Re: [PATCH] debug: fix BUILD_BUG_ON() for non-constant expressions
Date: Wed, 20 Aug 2008 16:21:05 +0300	[thread overview]
Message-ID: <48AC1A41.60907@panasas.com> (raw)
In-Reply-To: <20080817103241.GB21303@elte.hu>

Ingo Molnar wrote:
> From: Ingo Molnar <mingo@elte.hu>
> Date: Sun, 17 Aug 2008 12:18:01 +0200
> Subject: [PATCH] debug, x86: move BUILD_BUG_ON() and __FUNCTION__
> 
> move BUILD_BUG_ON variants and the __FUNCTION__ definition from
> kernel.h to compiler.h.
> 
> Besides being the correct location for such trivial wrappers around
> compiler functionality, this also allows the removal of a duplicate
> (and now slighly incompatible) definition of BUILD_BUG_ON from
> arch/x86/boot/boot.h.
> 
> [ boot.h cannot just include kernel.h to pick up the new definition of
>   BUILD_BUG_ON(), as it is also built into user-space utilities on the
>   host system. ]
> 
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
>  arch/x86/boot/boot.h     |    3 ---
>  include/linux/compiler.h |   30 ++++++++++++++++++++++++++++++
>  include/linux/kernel.h   |   26 --------------------------
>  3 files changed, 30 insertions(+), 29 deletions(-)
> 
> diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
> index 616b804..f09b79a 100644
> --- a/arch/x86/boot/boot.h
> +++ b/arch/x86/boot/boot.h
> @@ -27,9 +27,6 @@
>  #include "bitops.h"
>  #include <asm/cpufeature.h>
>  
> -/* Useful macros */
> -#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> -
>  extern struct setup_header hdr;
>  extern struct boot_params boot_params;
>  
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index c8bd2da..727862f 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -194,4 +194,34 @@ extern void __chk_io_ptr(const volatile void __iomem *);
>   */
>  #define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
>  
> +/*
> + * Force a compilation error if condition is true [array index becomes
> + * negative], and a linker error if condition is not constant [non-defined
> + * variable is used as an array index]:
> + *
> + * ( The linker trick relies on gcc optimizing out a multiplication with
> + *   constant zero - which should be reasonable enough. )
> + */
> +#ifndef __ASSEMBLY__
> +extern unsigned int __BUILD_BUG_ON_non_constant;
> +#endif
> +
> +#define BUILD_BUG_ON(condition)					\
> +do {								\
> +	(void)sizeof(char[1 - 2*!!(condition)]);		\
> +	if (!__builtin_constant_p(condition))			\
> +		__BUILD_BUG_ON_non_constant++;			\
> +} while (0)
> +
> +/*
> + * Force a compilation error if condition is true, but also produce a
> + * result (of value 0 and type size_t), so the expression can be used
> + * e.g. in a structure initializer (or where-ever else comma expressions
> + * aren't permitted):
> + */
> +#define BUILD_BUG_ON_ZERO(e) (sizeof(char[1 - 2 * !!(e)]) - 1)
> +
> +/* Trap pasters of __FUNCTION__ at compile-time */
> +#define __FUNCTION__ (__func__)
> +
>  #endif /* __LINUX_COMPILER_H */
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 36c841e..1ceafa4 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -467,32 +467,6 @@ struct sysinfo {
>  	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
>  };
>  
> -/*
> - * Force a compilation error if condition is true [array index becomes
> - * negative], and a linker error if condition is not constant [non-defined
> - * variable is used as an array index]:
> - *
> - * ( The linker trick relies on gcc optimizing out a multiplication with
> - *   constant zero - which should be reasonable enough. )
> - */
> -extern unsigned int __BUILD_BUG_ON_non_constant;
> -
> -#define BUILD_BUG_ON(condition)					\
> -do {								\
> -	(void)sizeof(char[1 - 2*!!(condition)]);		\
> -	if (!__builtin_constant_p(condition))			\
> -		__BUILD_BUG_ON_non_constant++;			\
> -} while (0)
> -
> -/* Force a compilation error if condition is true, but also produce a
> -   result (of value 0 and type size_t), so the expression can be used
> -   e.g. in a structure initializer (or where-ever else comma expressions
> -   aren't permitted). */
> -#define BUILD_BUG_ON_ZERO(e) (sizeof(char[1 - 2 * !!(e)]) - 1)
> -
> -/* Trap pasters of __FUNCTION__ at compile-time */
> -#define __FUNCTION__ (__func__)
> -
>  /* This helps us to avoid #ifdef CONFIG_NUMA */
>  #ifdef CONFIG_NUMA
>  #define NUMA_BUILD 1
> --
Ingo Hi!

I got bitten by this duplication too. Please submit an equivalent patch
for today's code. That could be nice.

Then later we can take care of BUILD_BUG_ON which ever variant.

Thanks
Boaz

  parent reply	other threads:[~2008-08-20 13:21 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-16 10:09 [PATCH] BUILD_BUG_ON sucks Alexey Dobriyan
2008-08-16 10:55 ` Rusty Russell
2008-08-16 20:07   ` Linus Torvalds
2008-08-17 10:32     ` [PATCH] debug: fix BUILD_BUG_ON() for non-constant expressions Ingo Molnar
2008-08-17 16:56       ` Linus Torvalds
2008-08-17 17:33         ` Ingo Molnar
2008-08-17 17:53           ` Ingo Molnar
2008-08-17 18:39           ` Linus Torvalds
2008-08-17 18:45             ` Ingo Molnar
2008-08-18  1:09           ` Rusty Russell
2008-08-18  7:54             ` Ingo Molnar
2008-08-18  9:55               ` Boaz Harrosh
2008-08-18 12:32                 ` Boaz Harrosh
2008-08-19 13:34                 ` Ingo Molnar
2008-08-19 16:33                   ` Boaz Harrosh
2008-08-20 10:59                     ` Ingo Molnar
2008-08-20 12:31                       ` Boaz Harrosh
2008-08-20 12:39                         ` adobriyan
2008-08-20 13:07                           ` Boaz Harrosh
2008-08-21 12:17                         ` Ingo Molnar
2008-08-25  1:19                     ` Rusty Russell
2008-08-20 13:21       ` Boaz Harrosh [this message]
2008-08-16 17:46 ` [PATCH] BUILD_BUG_ON sucks Andrew Morton
2008-08-17 12:19   ` Theodore Tso
2008-08-17 16:33     ` Andrew Morton

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=48AC1A41.60907@panasas.com \
    --to=bharrosh@panasas.com \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=rusty@rustcorp.com.au \
    --cc=sam@ravnborg.org \
    --cc=torvalds@linux-foundation.org \
    --cc=torvalds@linuxfoundation.org \
    /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.