Re: [Qemu-devel] [PATCH, RFC] Replace assert(0) with abort() or cpu_abort()

[Markus Armbruster]

Blue Swirl <address@hidden> writes:

> On 3/15/10, Markus Armbruster <address@hidden> wrote:
>> Blue Swirl <address@hidden> writes:
>>
>>  > When building with -DNDEBUG, assert(0) will not stop execution
>>  > so it must not be used for abnormal termination.
>>
>>
>> For each case: are you sure the code does not recover after assert(0)?
>>  Not saying it does, just asking whether you checked.
>
> I had not checked but did just now,
>
> QEMU system emulators do not handle SIGABRT. The situation is
> different for user emulator, but then assert(0) and abort() would be
> equally correct or incorrect.

Please don't tell me that user emulators make abort() return.  abort()
is declared __noreturn__, and the optimizer may well rely on that.

>>  > Use cpu_abort() when in CPU context, abort() otherwise.
>>
>>
>> I sympathize with the general idea, but I don't like dead code after
>>  abort().  What about cleaning that up?
>
> Good idea, but it should be a separate patch. This patch is "safe",
> whereas the cleanup patch could cause problems if it's not done
> carefully.

I support keeping separate things separate.  However, separating
something like

                    if (mapping->first_mapping_index != first_mapping_index
                            && mapping->info.file.offset > 0) {
-                       assert(0);
+                        abort();
                        copy_it = 1;
                    }

from

                    if (mapping->first_mapping_index != first_mapping_index
                            && mapping->info.file.offset > 0) {
                        abort();
-                       copy_it = 1;
                    }

doesn't seem worth it.