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

[Thomas Huth]

On 2018-10-18 19:28, Mark Cave-Ayland wrote:
> From: Laurent Vivier <address@hidden>
> 
> Co-developed-by: Mark Cave-Ayland <address@hidden>
> Signed-off-by: Mark Cave-Ayland <address@hidden>
> Signed-off-by: Laurent Vivier <address@hidden>
> ---
[...]
> +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.

> +    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?

 Thomas