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

Bernhard Beschow posted 12 patches 11 months, 2 weeks ago
Maintainers: Juan Quintela <quintela@redhat.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Leonardo Bras <leobras@redhat.com>, John Snow <jsnow@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, BALATON Zoltan <balaton@eik.bme.hu>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>, David Hildenbrand <david@redhat.com>
There is a newer version of this series
[PATCH v2 01/12] hw/block/fdc-isa: Free struct FDCtrl from PortioList
Posted by Bernhard Beschow 11 months, 2 weeks ago
FDCtrl::portio_list isn't used inside FDCtrl context but only inside
FDCtrlISABus context, so more it there.

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");
 
-- 
2.43.0
Re: [PATCH v2 01/12] hw/block/fdc-isa: Free struct FDCtrl from PortioList
Posted by BALATON Zoltan 11 months, 1 week ago
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");
>
>