Re: [PATCH v2 01/12] hw/block/fdc-isa: Free struct FDCtrl from PortioList

[BALATON Zoltan <balaton@eik.bme.hu>]

On Mon, 18 Dec 2023, Bernhard Beschow wrote:
> FDCtrl::portio_list isn't used inside FDCtrl context but only inside
> FDCtrlISABus context, so more it there.

"more" -> "move", you have the same typo in several other commit messages. 
Not sure I like the C++ism FDCtrl::portio_list and would write out "The 
portio_list field of FDCtrl" instead but not a big deal. Also the subject 
could say "Move portio_list from FDCtrl to FDCtrlISABus" which is less 
ambiguous than using free that's ususally associated with freeing memory.
Otherwise

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
> hw/block/fdc-internal.h | 2 --
> hw/block/fdc-isa.c      | 4 +++-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/block/fdc-internal.h b/hw/block/fdc-internal.h
> index 036392e9fc..fef2bfbbf5 100644
> --- a/hw/block/fdc-internal.h
> +++ b/hw/block/fdc-internal.h
> @@ -26,7 +26,6 @@
> #define HW_BLOCK_FDC_INTERNAL_H
>
> #include "exec/memory.h"
> -#include "exec/ioport.h"
> #include "hw/block/block.h"
> #include "hw/block/fdc.h"
> #include "qapi/qapi-types-block.h"
> @@ -140,7 +139,6 @@ struct FDCtrl {
>     /* Timers state */
>     uint8_t timer0;
>     uint8_t timer1;
> -    PortioList portio_list;
> };
>
> extern const FDFormat fd_formats[];
> diff --git a/hw/block/fdc-isa.c b/hw/block/fdc-isa.c
> index 7ec075e470..b4c92b40b3 100644
> --- a/hw/block/fdc-isa.c
> +++ b/hw/block/fdc-isa.c
> @@ -42,6 +42,7 @@
> #include "sysemu/block-backend.h"
> #include "sysemu/blockdev.h"
> #include "sysemu/sysemu.h"
> +#include "exec/ioport.h"
> #include "qemu/log.h"
> #include "qemu/main-loop.h"
> #include "qemu/module.h"
> @@ -60,6 +61,7 @@ struct FDCtrlISABus {
>     uint32_t irq;
>     uint32_t dma;
>     struct FDCtrl state;
> +    PortioList portio_list;
>     int32_t bootindexA;
>     int32_t bootindexB;
> };
> @@ -91,7 +93,7 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp)
>     FDCtrl *fdctrl = &isa->state;
>     Error *err = NULL;
>
> -    isa_register_portio_list(isadev, &fdctrl->portio_list,
> +    isa_register_portio_list(isadev, &isa->portio_list,
>                              isa->iobase, fdc_portio_list, fdctrl,
>                              "fdc");
>
>