Re: [PATCH v2] hw/audio/pcspk: Inline pcspk_init()

[Markus Armbruster <armbru@redhat.com>]

Bernhard Beschow <shentey@gmail.com> writes:

> Am 19. Oktober 2023 07:33:07 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>pcspk_init() is a legacy init function, inline and remove it.
>>
>>Since the device is realized using &error_fatal, use the same
>>error for setting the "pit" link.
>>
>>Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[...]

>>diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
>>index 63e0857208..79ffbb52a0 100644
>>--- a/hw/isa/i82378.c
>>+++ b/hw/isa/i82378.c
>>@@ -67,6 +67,7 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
>>     uint8_t *pci_conf;
>>     ISABus *isabus;
>>     ISADevice *pit;
>>+    ISADevice *pcspk;
>> 
>>     pci_conf = pci->config;
>>     pci_set_word(pci_conf + PCI_COMMAND,
>>@@ -102,7 +103,9 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
>>     pit = i8254_pit_init(isabus, 0x40, 0, NULL);
>> 
>>     /* speaker */
>>-    pcspk_init(isa_new(TYPE_PC_SPEAKER), isabus, pit);
>>+    pcspk = isa_new(TYPE_PC_SPEAKER);
>>+    object_property_set_link(OBJECT(pcspk), "pit", OBJECT(pit), &error_fatal);
>>+    isa_realize_and_unref(pcspk, isabus, &error_fatal);
>
> Why not pass errp here? I think that was Mark's comment in v1.

&error_fatal is almost always wrong in a function that takes Error **.
Happy to explain in more detail if needed.

[...]