Re: [PATCH v3 0/9] hw/nvram: hw/arm: Introduce Xilinx eFUSE and BBRAM

[Peter Maydell]

On Fri, 17 Sept 2021 at 06:24, Tong Ho <tong.ho@xilinx.com> wrote:
>
> This series implements the Xilinx eFUSE and BBRAM devices for
> the Versal and ZynqMP product families.
>
> Furthermore, both new devices are connected to the xlnx-versal-virt
> board and the xlnx-zcu102 board.
>
> See changes in docs/system/arm/xlnx-versal-virt.rst for detail.

Hi -- now this has landed upstream, Coverity points out a
lot of memory leaks on error or logging paths, where
the code does things like:

*** CID 1464071:  Resource leaks  (RESOURCE_LEAK)
/qemu/hw/nvram/xlnx-versal-efuse-ctrl.c: 628 in efuse_ctrl_reg_write()
622         dev = reg_array->mem.owner;
623         assert(dev);
624
625         s = XLNX_VERSAL_EFUSE_CTRL(dev);
626
627         if (addr != A_WR_LOCK && s->regs[R_WR_LOCK]) {
>>>     CID 1464071:  Resource leaks  (RESOURCE_LEAK)
>>>     Failing to save or free storage allocated by 
>>> "object_get_canonical_path((Object *)s)" leaks it.
628             qemu_log_mask(LOG_GUEST_ERROR,
629                           "%s[reg_0x%02lx]: Attempt to write
locked register.\n",
630                           object_get_canonical_path(OBJECT(s)), (long)addr);
631         } else {
632             register_write_memory(opaque, addr, data, size);
633         }

You need to free the memory here. A good pattern is how it's
done in xlnx-zynqmp-can.c with g_autofree:

    if (ARRAY_FIELD_EX32(s->regs, SOFTWARE_RESET_REGISTER, SRST)) {
        g_autofree char *path = object_get_canonical_path(OBJECT(s));

        qemu_log_mask(LOG_GUEST_ERROR, "%s: Attempting to transfer data while"
                      " data while controller is in reset mode.\n",
                      path);
        return false;
    }

Could somebody send some followup patches that fix all of these,
please? (There are 10 coverity issues, covering probably all of
the places where object_get_canonical_path() is used in this series.)

thanks
-- PMM