Re: [PATCH v2 0/7] Register API leaks fixes

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 9/10/25 08:23, Philippe Mathieu-Daudé wrote:
> Hi Luc,
> 
> On 2/10/25 09:34, Luc Michel wrote:
> 
>> This series addresses the memory leaks caused by the register API. The
>> first patches fix the API itself while the last ones take care of the
>> CANFD model.
> 
> 
>> Luc Michel (7):
>>    hw/core/register: remove the REGISTER device type
>>    hw/core/register: add the REGISTER_ARRAY type
>>    hw/core/register: remove the calls to `register_finalize_block'
>>    hw/core/register: remove the `register_finalize_block' function
>>    hw/net/can/xlnx-versal-canfd: remove unused include directives
>>    hw/net/can/xlnx-versal-canfd: refactor the banked registers logic
>>    hw/net/can/xlnx-versal-canfd: remove register API usage for banked
>>      regs
> 
> I had few issues with your series:
> 
> ../../hw/net/can/xlnx-versal-canfd.c:1917:30: error: unused variable 
> 'canfd_regs_ops' [-Werror,-Wunused-const-variable]
>   1917 | static const MemoryRegionOps canfd_regs_ops = {
>        |                              ^~~~~~~~~~~~~~
> 
> ../../hw/net/can/xlnx-versal-canfd.c:1871:42: error: use of undeclared 
> identifier 'TYPE_REGISTER'
>   1871 |         object_initialize(r, sizeof(*r), TYPE_REGISTER);
>        |                                          ^
> 
> ../../hw/net/can/xlnx-versal-canfd.c:1700:48: error: incompatible 
> pointer types passing 'hwaddr *' (aka 'unsigned long long *') to 
> parameter of type 'size_t *' (aka 'unsigned long *') [-Werror,- 
> Wincompatible-pointer-types]
>   1700 |     if (!canfd_decode_addr(s, addr, &bank_idx, &reg_offset)) {
>        |                                                ^~~~~~~~~~~
> ../../hw/net/can/xlnx-versal-canfd.c:1651:52: note: passing argument to 
> parameter 'offset' here
>   1651 |                               size_t *idx, size_t *offset)
>        |                                                    ^
> ../../hw/net/can/xlnx-versal-canfd.c:1722:48: error: incompatible 
> pointer types passing 'hwaddr *' (aka 'unsigned long long *') to 
> parameter of type 'size_t *' (aka 'unsigned long *') [-Werror,- 
> Wincompatible-pointer-types]
>   1722 |     if (!canfd_decode_addr(s, addr, &bank_idx, &reg_offset)) {
>        |                                                ^~~~~~~~~~~
> ../../hw/net/can/xlnx-versal-canfd.c:1651:52: note: passing argument to 
> parameter 'offset' here
>   1651 |                               size_t *idx, size_t *offset)
>        |                                                    ^
> 
> I fixed them by re-ordering the xlnx-versal-canfd patches first,
> having canfd_decode_FOO() taking a 'hwaddr *offset' and using a
> temporary __attribute__ ((unused)) for canfd_regs_ops[].
> 
> I'm queuing this series as fixed, except if you disagree.

Bah, qtest/device-introspect-test is failing:

# Testing device 'xlnx-zynqmp-efuse'
Broken pipe
../../tests/qtest/libqtest.c:208: kill_qemu() detected QEMU death from 
signal 11 (Segmentation fault: 11)
Abort trap: 6

(lldb) bt
* thread #5, stop reason = EXC_BAD_ACCESS (code=1, address=0x50)
   * frame #0: 0x00000001007dcf18 
qemu-system-aarch64`object_finalize_child_property(obj=<unavailable>, 
name=<unavailable>, opaque=0x00000001561f0c90) at object.c:1814:23
     frame #1: 0x00000001007d9ebc 
qemu-system-aarch64`object_property_del_all(obj=0x0000000138008000) at 
object.c:667:21 [inlined]
     frame #2: 0x00000001007d9e1c 
qemu-system-aarch64`object_finalize(data=0x0000000138008000) at 
object.c:728:5 [inlined]
     frame #3: 0x00000001007d9e14 
qemu-system-aarch64`object_unref(objptr=0x0000000138008000) at 
object.c:1232:9
     frame #4: 0x00000001008af32c 
qemu-system-aarch64`qmp_device_list_properties(typename=<unavailable>, 
errp=0x000000017002ac98) at qom-qmp-cmds.c:237:5
     frame #5: 0x000000010092d7a4 
qemu-system-aarch64`qmp_marshal_device_list_properties(args=0x0000000145809400, 
ret=0x0000000104827a18, errp=0x0000000104827a20) at 
qapi-commands-qdev.c:65:14
     frame #6: 0x0000000100948784 
qemu-system-aarch64`do_qmp_dispatch_bh(opaque=0x00000001048279e8) at 
qmp-dispatch.c:128:5

I'm dropping this series, please have a look.

Stashed changes:

-- >8 --
diff --git a/hw/net/can/xlnx-versal-canfd.c b/hw/net/can/xlnx-versal-canfd.c
index 81615bc52a6..d559fc06804 100644
--- a/hw/net/can/xlnx-versal-canfd.c
+++ b/hw/net/can/xlnx-versal-canfd.c
@@ -1784,3 +1784,3 @@ static bool 
canfd_decode_reg_bank(XlnxVersalCANFDState *s, hwaddr addr,
                                    hwaddr first_reg, hwaddr last_reg,
-                                  size_t num_banks, size_t *idx, size_t 
*offset)
+                                  size_t num_banks, size_t *idx, hwaddr 
*offset)
  {
@@ -1809,3 +1809,3 @@ static bool 
canfd_decode_reg_bank(XlnxVersalCANFDState *s, hwaddr addr,
  static bool canfd_decode_addr(XlnxVersalCANFDState *s, hwaddr addr,
-                              size_t *idx, size_t *offset)
+                              size_t *idx, hwaddr *offset)
  {
@@ -1916,2 +1916,3 @@ static const MemoryRegionOps canfd_ops = {

+__attribute__ ((unused))
  static const MemoryRegionOps canfd_regs_ops = {
---