Re: [PATCH] hw/pci-host/grackle: Verify PIC link is properly set

[Markus Armbruster <armbru@redhat.com>]

Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> On 10/12/20 1:50 PM, BALATON Zoltan via wrote:
>> On Mon, 12 Oct 2020, David Gibson wrote:
>>> On Mon, Oct 12, 2020 at 08:21:41AM +0200, Philippe
>>> Mathieu-Daudé wrote:
>>>> On 10/12/20 12:34 AM, David Gibson wrote:
>>>>> On Sun, Oct 11, 2020 at 09:03:32PM +0200, Philippe
>>>>> Mathieu-Daudé wrote:
>>>>>> The Grackle PCI host model expects the interrupt controller
>>>>>> being set, but does not verify it is present. Add a check to
>>>>>> help developers using this model.
>>>>>
>>>>> I don't think thaqt's very likely, but, sure, applied to ppc-for-5.2
>>>>
>>>> Do you want I correct the description as:
>>>> "The Grackle PCI host model expects the interrupt controller
>>>> being set, but does not verify it is present.
>>>> A developer basing its implementation on the Grackle model
>>>> might hit this problem. Add a check to help future developers
>>>> using this model as reference."?
>>>
>>> No, it's fine.  All I was saying is that the chances of anyone using
>>> Grackle in future seem very low to me.
>> So maybe an assert instead of a user visible error would be enough?
>
> My understanding is realize() shouldn't abort()
> (the caller might choose to by using &error_abort).

assert() is for checking invariants.  A violated invariant is a
programming error: developers screwed up, safe recovery is impossible.

Abusing assert() to catch errors that are not programming errors is
wrong.

You may check invariants with assert() anywhere in the code.

You should not misuse assert() anywhere in the code.

Sometimes, an error condition that is *not* a programming error in the
function where it is detected *is* a programming error for certain
calls.  Having these calls pass &error_abort is a common solution for
this problem.

Hope this helps.