[Qemu-devel] Re: [PATCH 2/2] Add __noreturn function attribute

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 2307 bytes --]

Jan Kiszka wrote:
> Anthony Liguori wrote:
>> Jan Kiszka wrote:
>>> Thiemo Seufer wrote:
>>>  
>>>> Jan Kiszka wrote:
>>>>    
>>>>> Avi Kivity wrote:
>>>>>      
>>>>>> Jan Kiszka wrote:
>>>>>>        
>>>>>>>> Breaking the standard is what brings us the joys of recently
>>>>>>>> (re)posted
>>>>>>>> patch for NetBSD and [u]intXX fun.
>>>>>>>>                 
>>>>>>> I have no problem with calling it 'noreturn' instead.
>>>>>>>             
>>>>>> That will break code that wants to use 'noreturn' as a local
>>>>>> variable. I think ATTR_NORETURN, while a lot uglier, is safer.
>>>>>>         
>>>>> Do you have such code already? Is it exported beyond qemu scope? Then
>>>>> why not going for our own convention "'noreturn' is reserved as
>>>>> function
>>>>> attribute"? (And yes, your macro is ugly :) ).
>>>>>       
>>>> "Macro names should be in upper case" is also a useful convention.
>>>>     
>>> Generally yes. But there are exceptions when the macro is used in a
>>> context where upper case disturbs the readability instead of improving
>>> it. I would argue that this is the case here, but it's always a matter
>>> of taste.
>>>
>>>  
>>>> FWIW, I agree with Stefan, there's currently not much need to isolate
>>>> gcc-isms.
>>>>     
>>> If everyone prefers having __attribute__ in the function prototypes
>>> directly -- OK. All I want is to get rid of the warnings without
>>> changing the code into the wrong direction.
>>>   
>> Please stick with the #define.  It's not about the GCC-ism, it's being
>> able to quickly replace it with something else.
> 
> For sure. Err... but which one now? "noreturn" is already available as
> signed patch.

...but that patch in fact has an issue with existing
__attribute__((noreturn)) sites. Converting them all to the common macro
raises the question where to put the definition. qemu-common.h appears
logical on first sight, but due to dyngen-exec.h's redefinition mess we
cannot include that header easily.

I'm currently trying to find a workaround that is not too invasive and
can quickly be removed once dyngen is gone. The deeper I dig for that,
the more weird the dependencies get.

However, please let me know if the preferred macro name is different
from "noreturn".

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]