lkml.org 
[lkml]   [2009]   [Dec]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] BUG(): CONFIG_BUG=n version of BUG() should be unreachable()
David Daney wrote:
> Alexander Beregalov wrote:
>> Previouss definition of BUG() as 'do {} while(0)' produced compilation
>> warnings when BUG() was used in default branch of switch() statement
>> (control reaches end of non-void function).
>>
>> Example:
>> unsigned long function()
>> {
>> switch() {
>> case 1:
>> return 1;
>> case 2:
>> return 2;
>> default:
>> BUG();
>> }
>>
>> Using unreachable() fixes the problem.
>>
>> Signed-off-by: Alexander Beregalov <a.beregalov@gmail.com>
>>
>
> NAK.
>

Well that may be too strong an objection, but I would really recommend
deeper consideration.

If you use: #define BUG() __builtin_unreachable()

which is what your patch does for GCC >= 4.5, it is truly undefined what
happens if it is ever reached. One typical thing that might happen is
that you start to try to execute data. It is unclear to me if it is
preferable in the kernel to do that, rather than loop endlessly. You
would likely achieve smaller code, but at what cost?

David Daney

>
>> diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
>> index 18c435d..1106439 100644
>> --- a/include/asm-generic/bug.h
>> +++ b/include/asm-generic/bug.h
>> @@ -89,7 +89,7 @@ extern void warn_slowpath_null(const char *file,
>> const int line);
>>
>> #else /* !CONFIG_BUG */
>> #ifndef HAVE_ARCH_BUG
>> -#define BUG() do {} while(0)
>> +#define BUG() unreachable() #endif
>>
>> #ifndef HAVE_ARCH_BUG_ON
>
> You can only use 'unreachable()' in situations where it is truly
> unreachable. In the case above you will reach it in the default case.
>
> I would suggest one of the following instead:
>
> #define BUG() BUILD_BUG_ON(1)
>
> #define BUG() do {} while(1)
>
>
> David Daney
>



\
 
 \ /
  Last update: 2009-12-23 02:39    [W:0.063 / U:0.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site