[Qemu-devel] [PATCH 05/67] migration: drop an unused include, add a used one

Michael S. Tsirkin posted 67 patches 7 years, 5 months ago
[Qemu-devel] [PATCH 05/67] migration: drop an unused include, add a used one
Posted by Michael S. Tsirkin 7 years, 5 months ago
we just need a struct name, let's add a forward
declaration instead of an include.

We also use size_t, so add stddef.h

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/migration/vmstate.h | 4 +++-
 migration/savevm.c          | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index df463fd..5103f15 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -27,7 +27,9 @@
 #ifndef QEMU_VMSTATE_H
 #define QEMU_VMSTATE_H
 
-#include "migration/qjson.h"
+#include <stddef.h>
+
+typedef struct QJSON QJSON;
 
 typedef struct VMStateInfo VMStateInfo;
 typedef struct VMStateDescription VMStateDescription;
diff --git a/migration/savevm.c b/migration/savevm.c
index e2be02a..532e22e 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -55,6 +55,7 @@
 #include "io/channel-buffer.h"
 #include "io/channel-file.h"
 #include "sysemu/replay.h"
+#include "qjson.h"
 
 #ifndef ETH_P_RARP
 #define ETH_P_RARP 0x8035
-- 
MST


Re: [Qemu-devel] [PATCH 05/67] migration: drop an unused include, add a used one
Posted by Eric Blake 7 years, 5 months ago
On 05/03/2018 02:50 PM, Michael S. Tsirkin wrote:
> we just need a struct name, let's add a forward
> declaration instead of an include.

Reasonable.

> 
> We also use size_t, so add stddef.h

Why? osdep.h already does this, and ALL .c files that use include/ 
should be including osdep.h prior to any other in-tree .h file.  So 
size_t should already be in scope by the time this header is included.

> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>   include/migration/vmstate.h | 4 +++-
>   migration/savevm.c          | 1 +
>   2 files changed, 4 insertions(+), 1 deletion(-)
-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 05/67] migration: drop an unused include, add a used one
Posted by Michael S. Tsirkin 7 years, 5 months ago
On Thu, May 03, 2018 at 03:02:59PM -0500, Eric Blake wrote:
> On 05/03/2018 02:50 PM, Michael S. Tsirkin wrote:
> > we just need a struct name, let's add a forward
> > declaration instead of an include.
> 
> Reasonable.
> 
> > 
> > We also use size_t, so add stddef.h
> 
> Why? osdep.h already does this, and ALL .c files that use include/ should be
> including osdep.h prior to any other in-tree .h file.  So size_t should
> already be in scope by the time this header is included.

It seemed cleaner to have the file self-contained.
OK, I will drop this.
There's a small number of files which do not include qemu/osdep.h.
Might be worth fixing.

> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >   include/migration/vmstate.h | 4 +++-
> >   migration/savevm.c          | 1 +
> >   2 files changed, 4 insertions(+), 1 deletion(-)
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 05/67] migration: drop an unused include, add a used one
Posted by Eric Blake 7 years, 5 months ago
On 05/03/2018 03:15 PM, Michael S. Tsirkin wrote:
> On Thu, May 03, 2018 at 03:02:59PM -0500, Eric Blake wrote:
>> On 05/03/2018 02:50 PM, Michael S. Tsirkin wrote:
>>> we just need a struct name, let's add a forward
>>> declaration instead of an include.
>>
>> Reasonable.
>>
>>>
>>> We also use size_t, so add stddef.h
>>
>> Why? osdep.h already does this, and ALL .c files that use include/ should be
>> including osdep.h prior to any other in-tree .h file.  So size_t should
>> already be in scope by the time this header is included.
> 
> It seemed cleaner to have the file self-contained.
> OK, I will drop this.
> There's a small number of files which do not include qemu/osdep.h.
> Might be worth fixing.

scripts/clean-includes is already supposed to fix these.

There are a few .c files exempt from including osdep.h, but in general, 
those files should probably also not be including anything from include/.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 05/67] migration: drop an unused include, add a used one
Posted by Michael S. Tsirkin 7 years, 5 months ago
On Thu, May 03, 2018 at 03:18:47PM -0500, Eric Blake wrote:
> On 05/03/2018 03:15 PM, Michael S. Tsirkin wrote:
> > On Thu, May 03, 2018 at 03:02:59PM -0500, Eric Blake wrote:
> > > On 05/03/2018 02:50 PM, Michael S. Tsirkin wrote:
> > > > we just need a struct name, let's add a forward
> > > > declaration instead of an include.
> > > 
> > > Reasonable.
> > > 
> > > > 
> > > > We also use size_t, so add stddef.h
> > > 
> > > Why? osdep.h already does this, and ALL .c files that use include/ should be
> > > including osdep.h prior to any other in-tree .h file.  So size_t should
> > > already be in scope by the time this header is included.
> > 
> > It seemed cleaner to have the file self-contained.
> > OK, I will drop this.
> > There's a small number of files which do not include qemu/osdep.h.
> > Might be worth fixing.
> 
> scripts/clean-includes is already supposed to fix these.
> 
> There are a few .c files exempt from including osdep.h, but in general,
> those files should probably also not be including anything from include/.

Here's a list from a quick grep.
Most of them probably get osdep.h indirectly.


+contrib/libvhost-user/libvhost-user.c
+hw/rdma/rdma_utils.c
+libuser/trace.c
+target/mips/translate_init.c
+target/ppc/mfrom_table.c
+target/ppc/translate/dfp-impl.inc.c
+target/ppc/translate/dfp-ops.inc.c
+target/ppc/translate/fp-impl.inc.c
+target/ppc/translate/fp-ops.inc.c
+target/ppc/translate/spe-impl.inc.c
+target/ppc/translate/spe-ops.inc.c
+target/ppc/translate/vmx-impl.inc.c
+target/ppc/translate/vmx-ops.inc.c
+target/ppc/translate/vsx-impl.inc.c
+target/ppc/translate/vsx-ops.inc.c
+target/s390x/gen-features.c
+target/xtensa/core-dc232b/gdb-config.inc.c
+target/xtensa/core-dc233c/gdb-config.inc.c
+target/xtensa/core-de212/gdb-config.inc.c
+target/xtensa/core-sample_controller/gdb-config.inc.c
+tcg/aarch64/tcg-target.inc.c
+tcg/arm/tcg-target.inc.c
+tcg/i386/tcg-target.inc.c
+tcg/mips/tcg-target.inc.c
+tcg/ppc/tcg-target.inc.c
+tcg/s390/tcg-target.inc.c
+tcg/sparc/tcg-target.inc.c
+tcg/tcg-ldst.inc.c
+tcg/tcg-pool.inc.c
+tcg/tci/tcg-target.inc.c
+tests/multiboot/libc.c
+tests/multiboot/mmap.c
+tests/multiboot/modules.c

lots of files under tests/tcg

+tests/test-qapi-event.c
+tests/test-qapi-types.c
+tests/test-qapi-visit.c
+tests/test-qmp-introspect.c
+tests/test-qmp-marshal.c

+ui/keycodemapdb/tests/stdc.c


> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 05/67] migration: drop an unused include, add a used one
Posted by Eric Blake 7 years, 5 months ago
On 05/03/2018 03:29 PM, Michael S. Tsirkin wrote:

>>
>> There are a few .c files exempt from including osdep.h, but in general,
>> those files should probably also not be including anything from include/.
> 
> Here's a list from a quick grep.
> Most of them probably get osdep.h indirectly.
> 
> 
> +contrib/libvhost-user/libvhost-user.c

contrib may or may not be exempt; but it's not in the whitelist.

> +hw/rdma/rdma_utils.c

Probably a bug; looks like a recently added file.

> +libuser/trace.c

Huh? I don't see this file in git.

> +target/mips/translate_init.c
> +target/ppc/mfrom_table.c

Probably bugs.

> +target/ppc/translate/dfp-impl.inc.c

This, and all other .inc.c, are indeed picking it up indirectly (by the 
master .c that is including these secondary .c), so they are exempt. 
They are whitelisted by the script via the shell case statement.

> +target/s390x/gen-features.c

Possibly a bug.

> +tests/multiboot/libc.c
> +tests/multiboot/mmap.c
> +tests/multiboot/modules.c

Exempt, per the XDIRREGEX in scripts/clean-includes

> 
> lots of files under tests/tcg

Also exempt.

> 
> +tests/test-qapi-event.c

Huh? That starts with #include "qemu/osdep.h".  But it's not a file 
under version control.  In fact, it's stale leftovers (it used to be 
generated by that name, but now we generate tests/test-qapi-events.c.

> +tests/test-qapi-types.c
> +tests/test-qapi-visit.c
> +tests/test-qmp-introspect.c
> +tests/test-qmp-marshal.c

Again, how are you flagging these generated files?

> 
> +ui/keycodemapdb/tests/stdc.c

Exempt, since it's in a submodule.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org