Re: [Qemu-devel] [PULL 3/8] m68k: Add NeXTcube machine

[Peter Maydell <peter.maydell@linaro.org>]

On Sat, 7 Sep 2019 at 16:47, Thomas Huth <huth@tuxfamily.org> wrote:
>
> It is still quite incomplete (no SCSI, no floppy emulation, no network,
> etc.), but the firmware already shows up the debug monitor prompt in the
> framebuffer display, so at least the very basics are already working.
>
> This code has been taken from Bryce Lanham's GSoC 2011 NeXT branch at
>
>  https://github.com/blanham/qemu-NeXT/blob/next-cube/hw/next-cube.c
>
> and altered quite a bit to fit the latest interface and coding conventions
> of the current QEMU.
>
> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Message-Id: <20190831074519.32613-4-huth@tuxfamily.org>
> Signed-off-by: Thomas Huth <huth@tuxfamily.org

Hi; Coverity spotted an issue in this function
(CID 1405664):


> +static void nextscr2_write(NeXTState *s, uint32_t val, int size)
> +{
> +    static int led;
> +    static int phase;
> +    static uint8_t old_scr2;
> +    static uint8_t rtc_command;
> +    static uint8_t rtc_value;
> +    static uint8_t rtc_status = 0x90;
> +    static uint8_t rtc_return;
> +    uint8_t scr2_2;
> +


> +                /* read the status 0x31 */
> +                if (rtc_command == 0x31) {
> +                    scr2_2 = scr2_2 & (~SCR2_RTDATA);
> +                    /* for now 0x00 */
> +                    if (0x00 & (0x80 >> (phase - 8))) {

0 & anything can never be true, so the line below here is dead code.

> +                        scr2_2 |= SCR2_RTDATA;
> +                    }
> +                    rtc_return = (rtc_return << 1) |
> +                                 ((scr2_2 & SCR2_RTDATA) ? 1 : 0);
> +                }
> +

Incidentally, I see that this file has quite a lot of
what seems to be essentially device emulation code in it
(a bunch of IO MemoryRegions defined locally) -- ideally
these could be split out into proper device objects at
some point.

thanks
-- PMM