Re: [PATCH 02/15] digic: stash firmware into DigicState

[Paolo Bonzini <pbonzini@redhat.com>]

On 26/10/20 15:44, Peter Maydell wrote:
> Anyway, rather than stashing the firmware filename in the
> DigicState, you could lift the "decide whether to use
> machine->firmware or the def_filename" choice up to
> the callsites in digic4_board_init():
> 
>     if (board->add_rom0) {
>         board->add_rom0(s, DIGIC4_ROM0_BASE,
>                         machine->firmware ?: board->rom0_def_filename);
>     }
> (and similarly for rom1).
> 
> Then you can delete the
>     if (bios_name) {
>         filename = bios_name;
>     } else {
>         filename = def_filename;
>     }
> block from digic_load_rom() and rename the arguments of
> digic_load_rom() and digic4_add_k8p3215uqb_rom() to just
> "filename" rather than "def_filename".
> 
> Doing it that way avoids passing things around that we don't
> need to, and makes it clear in the digic4_board_init() code
> that we're doing something a bit suspect in possibly using
> the machine->firmware file twice -- if we ever need to fix
> that bug then it'll be a simple change to the logic in that
> one function.

Much better indeed, thanks!

Paolo