Re: [PATCH] hw/boards: Convert MachineClass bitfields to boolean

[Peter Maydell]

On Wed, 22 Jan 2025 at 12:36, Thomas Huth <thuth@redhat.com> wrote:
>
> On 22/01/2025 11.32, Philippe Mathieu-Daudé wrote:
> > As Daniel mentioned:
> >
> >   "The number of instances of MachineClass is not large enough
> >    that we save a useful amount of memory through bitfields."
> >
> > Also, see recent commit ecbf3567e21 ("docs/devel/style: add a
> > section about bitfield, and disallow them for packed structures").
> >
> > Convert the MachineClass bitfields used as boolean as real ones.
> >
> > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   include/hw/boards.h        | 14 +++++++-------
> >   hw/arm/aspeed.c            |  6 +++---
> >   hw/arm/fby35.c             |  4 ++--
> >   hw/arm/npcm7xx_boards.c    |  6 +++---
> >   hw/arm/raspi.c             |  6 +++---
> >   hw/arm/sbsa-ref.c          |  2 +-
> >   hw/arm/virt.c              |  2 +-
> >   hw/arm/xilinx_zynq.c       |  2 +-
> >   hw/avr/arduino.c           |  6 +++---
> >   hw/core/null-machine.c     | 10 +++++-----
> >   hw/i386/microvm.c          |  2 +-
> >   hw/i386/pc_piix.c          |  2 +-
> >   hw/i386/pc_q35.c           |  4 ++--
> >   hw/loongarch/virt.c        |  2 +-
> >   hw/m68k/virt.c             |  6 +++---
> >   hw/ppc/pnv.c               |  2 +-
> >   hw/ppc/spapr.c             |  2 +-
> >   hw/riscv/virt.c            |  2 +-
> >   hw/s390x/s390-virtio-ccw.c |  8 ++++----
> >   hw/xtensa/sim.c            |  2 +-
> >   20 files changed, 45 insertions(+), 45 deletions(-)
>
> So if you are touching all these files, why not go with an even more
> meaningful rework instead? Flip the meaning of the "no_*" flags to the
> opposite, so that we e.g. have "has_default_cdrom" instead of "no_cdrom",
> then new boards would not have to remember to set these ugly "no_" flags
> anymore. It's quite a bit of work, but it could certainly be helpful in the
> long run.

Well, that depends on what you think the default for new
boards should be. I suspect these are all no_foo because
when they were put in the idea was "all boards should
be default have a foo, and 'this board defaults to not
having a foo' is the rarer special case it has to set"...

-- PMM