Re: [Qemu-devel] [PATCH 6/7] mac_dbdma: change DBDMA_register_channel to a MAC_DBDMA type method

[Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>]

On 26/09/17 04:47, David Gibson wrote:

> On Sun, Sep 24, 2017 at 03:47:45PM +0100, Mark Cave-Ayland wrote:
>> Using this we can change the MACIO_IDE instance to register the channel
>> itself via a type method instead of requiring a separate
>> DBDMA_register_channel() function.
>>
>> As a consequence of this it is now possible to remove the old external
>> macio_ide_register_dma() function.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> Ok, two concerns about this.
> 
> First, you've added the function pointer to the instance, not to the
> class, which is not how a QOM method would normally be done.

Yeah I did think about whether I needed to create a full class but was
torn since as you say there is only one implementation.

> More generally, though, why is a method preferable to a plain
> function?  AFAICT it's not plausible that there will ever be more than
> one implementation of the method.
> 
> Same comments apply to patch 7/7.

For me it's really for encapsulation. It seems a little odd requiring a
global function to configure a QOM object to which I already have a
reference.

If I were to redo the last 2 patches using a proper class, would you
accept them?


ATB,

Mark.