qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH] hw/boards: Convert MachineClass bitfields to boolean
Date: Wed, 22 Jan 2025 17:58:38 +0100
User-agent: Mozilla Thunderbird

On 22/1/25 16:27, Thomas Huth wrote:
On 22/01/2025 15.33, Peter Maydell wrote:
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"...

That might have been the reasoning for the naming 20 years ago. But times have changed... which recent board does still have a floppy drive? parallel port? And the others are also not that common anymore...

I tried but not much interest:
https://lore.kernel.org/qemu-devel/219be312-4acc-46c9-8a56-16e5cb483ae4@linaro.org/

So Daniel suggested to clean the bad bitfield pattern in parallel.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]