qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH qom v3 1/4] cpu: Add wrapper to the set-pc() hoo


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH qom v3 1/4] cpu: Add wrapper to the set-pc() hook
Date: Thu, 25 Jun 2015 12:21:58 +0100

On 25 June 2015 at 12:12, Andreas Färber <address@hidden> wrote:
> Am 24.06.2015 um 21:11 schrieb Peter Maydell:
>> On 24 June 2015 at 19:09, Andreas Färber <address@hidden> wrote:
>>> +    g_assert(cc->set_pc != NULL);
>>> +    cc->set_pc(cpu, addr);
>>>  }
>>
>> Do we need this assert? If it would have fired
>> then we'll just crash immediately calling the null pointer,
>> so it's not like it's guarding against a more subtle failure
>> at a later point...
>
> There seemed uncertainty whether all corner cases of the 17 targets
> implement set_pc for all subclasses. By my reading, g_assert() calls
> g_assertion_message_expr(), which is marked G_GNUC_NORETURN - and I
> assume it to abort after printing the message, raising a signal and
> either exiting the process or falling back to an attached debugger.
>
> It may be unnecessary, but I don't see it calling the null pointer here.

What I mean is:
 * with the assert, QEMU will die in this function if cc->set_pc
   is NULL, in an easily debuggable way
 * without the assert, QEMU will still die in this function if
   cc->set_pc is NULL, in an easily debuggable way

So the assert doesn't hurt, but it doesn't really gain anything IMHO.
Assertions are most useful when they turn something that would be
a really confusing failure much later in execution into an easily
debuggable crash earlier on, I think.

Still, this is a very minor thing, so it's a personal taste/style
question, as you say. You can leave the assert in or remove it,
whichever you prefer.

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]