On 7/4/19 4:20 PM, Peter Maydell wrote: > In the virt machine, we support TrustZone being either present or > absent, and so the code must deal with the secure_sysmem pointer > possibly being NULL. In the sbsa-ref machine, TrustZone is always > present, but some code and comments copied from virt still treat > it as possibly not being present. > > This causes Coverity to complain (CID 1407287) that we check > secure_sysmem for being NULL after an unconditional dereference. > Simplify the code so that instead of initializing the variable > to NULL, unconditionally assigning it, and then testing it for NULL, > we just initialize it correctly in the variable declaration and > then assume it to be non-NULL. We also delete a comment which > only applied to the non-TrustZone config. > > Signed-off-by: Peter Maydell <[email protected]> > --- > Not a bug as such, but we should put it in for 4.1 to > keep Coverity happy. > --- > hw/arm/sbsa-ref.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c > index ee53f0ff60d..6f315b79445 100644 > --- a/hw/arm/sbsa-ref.c > +++ b/hw/arm/sbsa-ref.c > @@ -254,8 +254,6 @@ static void sbsa_flash_map(SBSAMachineState *sms, > * sysmem is the system memory space. secure_sysmem is the secure view > * of the system, and the first flash device should be made visible only > * there. The second flash device is visible to both secure and > nonsecure. > - * If sysmem == secure_sysmem this means there is no separate Secure > - * address space and both flash devices are generally visible. > */ > hwaddr flashsize = sbsa_ref_memmap[SBSA_FLASH].size / 2; > hwaddr flashbase = sbsa_ref_memmap[SBSA_FLASH].base; > @@ -588,7 +586,7 @@ static void sbsa_ref_init(MachineState *machine) > SBSAMachineState *sms = SBSA_MACHINE(machine); > MachineClass *mc = MACHINE_GET_CLASS(machine); > MemoryRegion *sysmem = get_system_memory(); > - MemoryRegion *secure_sysmem = NULL; > + MemoryRegion *secure_sysmem = g_new(MemoryRegion, 1); > MemoryRegion *ram = g_new(MemoryRegion, 1); > bool firmware_loaded; > const CPUArchIdList *possible_cpus; > @@ -612,13 +610,11 @@ static void sbsa_ref_init(MachineState *machine) > * containing the system memory at low priority; any secure-only > * devices go in at higher priority and take precedence. > */ > - secure_sysmem = g_new(MemoryRegion, 1); > memory_region_init(secure_sysmem, OBJECT(machine), "secure-memory", > UINT64_MAX); > memory_region_add_subregion_overlap(secure_sysmem, 0, sysmem, -1); > > - firmware_loaded = sbsa_firmware_init(sms, sysmem, > - secure_sysmem ?: sysmem); > + firmware_loaded = sbsa_firmware_init(sms, sysmem, secure_sysmem); > > if (machine->kernel_filename && firmware_loaded) { > error_report("sbsa-ref: No fw_cfg device on this machine, " >
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
