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

[Paolo Bonzini <pbonzini@redhat.com>]

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