Re: [Qemu-devel] [PATCH v2 1/3] hw/nvram/fw_cfg: Remove various typedefs from "qemu/typedefs.h"

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Jan 14, 2019 at 06:52:21PM +0100, Paolo Bonzini wrote:
> On 14/01/19 16:49, Michael S. Tsirkin wrote:
> > On Mon, Jan 14, 2019 at 02:08:27PM +0100, Philippe Mathieu-Daudé wrote:
> >> There are only three include files requiring these typedefs, let them
> >> include "hw/nvram/fw_cfg.h" directly to simplify "qemu/typedefs.h".
> >>
> >> To clean "qemu/typedefs.h", move the declarations to "hw/nvram/fw_cfg.h".
> >> Reorder two function declarations to avoid forward typedef declarations.
> >>
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> >> ---
> >>  include/hw/acpi/vmgenid.h |  1 +
> >>  include/hw/arm/virt.h     |  1 +
> >>  include/hw/mem/nvdimm.h   |  1 +
> >>  include/hw/nvram/fw_cfg.h | 22 ++++++++++++----------
> >>  include/qemu/typedefs.h   |  4 ----
> >>  5 files changed, 15 insertions(+), 14 deletions(-)
> > 
> > I am sorry I don't understand what this patchset is about.
> > Supposed to be a cleanup but instead it pulls
> > fw cfg into any user of unrelated headers.
> > 
> > More lines of code slower builds what's to like?
> > 
> > Shared typedefs is what typedefs.h is about.
> > These are clearly shared so let's keep it simple.
> 
> I can see why this patch can be a bit controversial.  Personally I think
> it's okay either way (which generally tends to favor the status quo).
> 
> Paolo

True - not something to lose sleep over.

-- 
MST