Re: [PULL v2 13/15] hw/rx: Add RX GDB simulator

[Philippe Mathieu-Daudé]

Hi Yoshinori,

On 9/7/20 3:13 PM, Peter Maydell wrote:
> On Mon, 22 Jun 2020 at 20:20, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> From: Yoshinori Sato <ysato@users.sourceforge.jp>
>>
>> Add the RX machine internally simulated in GDB.
> 
> Hi; Coverity points out a memory leak (CID 1432307) in this function:
> 
>> +static void rx_gdbsim_init(MachineState *machine)
>> +{
> 
>> +        if (dtb_filename) {
>> +            ram_addr_t dtb_offset;
>> +            int dtb_size;
>> +            void *dtb;
>> +
>> +            dtb = load_device_tree(dtb_filename, &dtb_size);
> 
> This allocates memory...
> 
>> +            if (dtb == NULL) {
>> +                error_report("Couldn't open dtb file %s", dtb_filename);
>> +                exit(1);
>> +            }
>> +            if (machine->kernel_cmdline &&
>> +                qemu_fdt_setprop_string(dtb, "/chosen", "bootargs",
>> +                                        machine->kernel_cmdline) < 0) {
>> +                error_report("Couldn't set /chosen/bootargs");
>> +                exit(1);
>> +            }
>> +            /* DTB is located at the end of SDRAM space. */
>> +            dtb_offset = machine->ram_size - dtb_size;
>> +            rom_add_blob_fixed("dtb", dtb, dtb_size,
>> +                               SDRAM_BASE + dtb_offset);
> 
> ...and rom_add_blob_fixed() copies that memory, it doesn't take
> ownership of it, so after we've called it we need to
>     g_free(fdt);

Can you send a patch to fix this please?

> 
>> +            /* Set dtb address to R1 */
>> +            RXCPU(first_cpu)->env.regs[1] = SDRAM_BASE + dtb_offset;
>> +        }
>> +    }
> 
> thanks
> -- PMM
>