On Sat, 12 Feb 2022 at 13:27, BALATON Zoltan <bala...@eik.bme.hu> wrote: > > On Sat, 12 Feb 2022, Bernhard Beschow wrote: > > Passing own DeviceState rather than just the IRQs allows for resolving > > global variables. > > Do you mean pci_set_irq_fn instead of pci_map_irq_fn in the patch title? > > > Signed-off-by: Bernhard Beschow <shen...@gmail.com> > > Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> > > Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org> > > --- > > hw/isa/piix4.c | 6 +++--- > > hw/pci-host/sh_pci.c | 6 +++--- > > hw/pci-host/versatile.c | 6 +++--- > > hw/ppc/ppc440_pcix.c | 6 +++--- > > hw/ppc/ppc4xx_pci.c | 6 +++--- > > 5 files changed, 15 insertions(+), 15 deletions(-) > > You don't seem to change any global function definition that reqires this > change in all these devices so why can't these decide on their own what > their preferred opaque data is for their set irq function and only change > piix4? Am I missing something?
Yeah, we don't necessarily need to change all these devices, but I think this is an area where over time we're shifting from an older school of API design that was "we have some basically standalone functions that take an arbitrary opaque pointer" towards a more object-oriented design, where the opaque pointer should probably be the pointer-to-this-object unless there's a good reason why it needs to be something else[*], because having a function that's part of a device being passed something other than the device-instance pointer is a bit unexpected. In some cases there is a good reason for opaque pointers for function callbacks to be something else, because passing in the device pointer would make the code noticeably more complicated. But in the specific cases here, the only change is that the callback functions use "s->somearray[i]" instead of "an_argument[i]", which doesn't seem to me like a significant complexity change. thanks -- PMM
