Re: [Qemu-devel] [PATCH v4 07/11] hw/m68k: add Nubus support

[Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>]

On 23/10/2018 08:36, Thomas Huth wrote:

> On 2018-10-18 19:28, Mark Cave-Ayland wrote:
>> From: Laurent Vivier <laurent@vivier.eu>
>>
>> Co-developed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>> ---
> [...]
>> +static void nubus_register_format_block(NubusDevice *dev)
>> +{
>> +    char fblock_name[27];
>> +
>> +    sprintf(fblock_name, "nubus-slot-%d-format-block", dev->slot_nb);
> 
> Latest GCC (version 8) got very picky about possible buffer overflows
> during sprintf() ... not sure, but it might be necessary to either use a
> bigger array here, or assert(dev->slot_nb < NUBUS_SLOT_NB), or even
> better use g_strdup_printf() instead (with g_free() at the end of the
> function) instead.

Good point. I've gone for replacing sprintf() with g_strdup_printf() whenever it
occurs in this file.

>> +    hwaddr fblock_offset = memory_region_size(&dev->slot_mem) - FBLOCK_SIZE;
>> +    memory_region_init_io(&dev->fblock_io, NULL, &nubus_format_block_ops,
>> +                          dev, fblock_name, FBLOCK_SIZE);
>> +    memory_region_add_subregion(&dev->slot_mem, fblock_offset,
>> +                                &dev->fblock_io);
>> +}
> [...]
>> diff --git a/include/hw/display/macfb.h b/include/hw/display/macfb.h
>> index 70ea5480fe..3059f2f36a 100644
>> --- a/include/hw/display/macfb.h
>> +++ b/include/hw/display/macfb.h
>> @@ -39,4 +39,25 @@ typedef struct {
>>      MacfbState macfb;
>>  } MacfbSysBusState;
>>  
>> +#define MACFB_NUBUS_DEVICE_CLASS(class) \
>> +    OBJECT_CLASS_CHECK(MacfbNubusDeviceClass, (class), TYPE_NUBUS_MACFB)
>> +#define MACFB_NUBUS_GET_CLASS(obj) \
>> +    OBJECT_GET_CLASS(MacfbNubusDeviceClass, (obj), TYPE_NUBUS_MACFB)
>> +
>> +typedef struct MacfbNubusDeviceClass {
>> +    DeviceClass parent_class;
>> +
>> +    DeviceRealize parent_realize;> +} MacfbNubusDeviceClass;
>>
>> +#define TYPE_NUBUS_MACFB "nubus-macfb"
>> +#define NUBUS_MACFB(obj) \
>> +    OBJECT_CHECK(MacfbNubusState, (obj), TYPE_NUBUS_MACFB)
>> +
>> +typedef struct {
>> +    NubusDevice busdev;
>> +
>> +    MacfbState macfb;
>> +} MacfbNubusState;
>> +
>>  #endif
> 
> I think this should rather be part of the next patch instead?

Ooops yes. I've fixed in the latest version of the patch series.


ATB,

Mark.