On Mon, 4 Oct 2021, Laurent Vivier wrote:
> Le 02/10/2021 à 12:33, Peter Maydell a écrit :
>> On Wed, 29 Sept 2021 at 10:49, Laurent Vivier <laurent@vivier.eu> wrote:
>>>
>>> From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>
>>> According to "Designing Cards and Drivers for the Macintosh Family" each physical
>>> nubus slot can access 2 separate address ranges: a super slot memory region which
>>> is 256MB and a standard slot memory region which is 16MB.
>>>
>>> Currently a Nubus device uses the physical slot number to determine whether it is
>>> using a standard slot memory region or a super slot memory region rather than
>>> exposing both memory regions for use as required.
>>
>>
>>> +    /* Super */
>>> +    slot_offset = nd->slot * NUBUS_SUPER_SLOT_SIZE;
>>
>> Hi; Coverity thinks this multiply might overflow, because
>> we're calculating a hw_addr (64-bits) but the multiply is only
>> done at 32-bits. Adding an explicit cast or using 'ULL' in the
>> constant #define rather than just 'U' would fix this.
>> This is CID 1464070.
>>
>
> I'm wondering if adding "assert(nd->slot < NUBUS_SUPER_SLOT_NB)" would help coverity to avoid the
> error without using 64bit arithmetic?

Using ULL in constant is simpler and better, assert is an unnecessary 
condition evaluation in cases where it can't happen (that's not a 
performance problem here but could be in some frequently called code).

Regards,
BALATON Zoltan

>
>>> +
>>> +    name = g_strdup_printf("nubus-super-slot-%x", nd->slot);
>>> +    memory_region_init(&nd->super_slot_mem, OBJECT(dev), name,
>>> +                       NUBUS_SUPER_SLOT_SIZE);
>>> +    memory_region_add_subregion(&nubus->super_slot_io, slot_offset,
>>> +                                &nd->super_slot_mem);
>>> +    g_free(name);
>>> +
>>> +    /* Normal */
>>> +    slot_offset = nd->slot * NUBUS_SLOT_SIZE;
>>
>> Same with this one.
>
> assert(nb->slot < NUBUS_SLOT_NB)
>
>> thanks
>> -- PMM
>>
>
> Laurent
>
>
>