8:41 a.m.
The public headers libvirt/libvirt.h, libvirt/virterror.h,
libvirt/libvirt-qemu.h, and libvirt/libvirt-lxc.h shouldn't
be included in internal source directly.
Osier Yang (10):
cleanup: Remove the duplicate header
cleanup: Only include testutils.h once
cleanup: Don't include libvirt/libvirt.h
cleanup: Don't include libvirt/virterror.h
syntax-check: Don't include duplicate header
syntax-check: Don't include "libvirt.h" and "virterror.h" in
"" form
syntax-check: Include libvirt.h and virterror.h in <> form only in
external tools
syntax-check: Don't include libvirt-{qemu,lxc}.h internally
syntax-check: Include libvirt-{qemu,lxc}.h in <> form only in external
tools
docs: Update hacking
cfg.mk | 106 ++++++++++++++++++++++++++++++++++++++
daemon/libvirtd.c | 1 -
daemon/remote.c | 2 -
docs/hacking.html.in | 8 +--
include/libvirt/libvirt-lxc.h | 2 +-
include/libvirt/libvirt-qemu.h | 2 +-
python/libvirt-lxc-override.c | 4 +-
python/libvirt-override.c | 4 +-
python/libvirt-qemu-override.c | 4 +-
python/typewrappers.h | 4 +-
src/conf/node_device_conf.c | 1 -
src/libvirt-qemu.c | 1 -
src/network/bridge_driver.c | 1 -
src/nodeinfo.h | 1 -
src/openvz/openvz_conf.c | 1 -
src/openvz/openvz_driver.c | 1 -
src/parallels/parallels_driver.c | 1 -
src/phyp/phyp_driver.c | 2 -
src/qemu/qemu_capabilities.h | 1 -
src/qemu/qemu_driver.c | 1 -
src/remote/remote_driver.h | 2 -
src/remote/remote_protocol.x | 1 -
src/security/security_selinux.c | 1 -
src/uml/uml_driver.c | 1 -
src/util/virkeycode.h | 1 -
src/util/virnetdevtap.c | 1 -
src/xen/xen_hypervisor.c | 1 -
tests/domainsnapshotxml2xmltest.c | 4 +-
tests/esxutilstest.c | 4 +-
tests/lxcxml2xmltest.c | 4 +-
tests/openvzutilstest.c | 4 +-
tests/qemuargv2xmltest.c | 4 +-
tests/qemuhelptest.c | 4 +-
tests/qemumonitortest.c | 4 +-
tests/qemuxml2argvtest.c | 4 +-
tests/qemuxml2xmltest.c | 4 +-
tests/qemuxmlnstest.c | 4 +-
tests/shunloadhelper.c | 2 -
tests/shunloadtest.c | 4 +-
tests/vmx2xmltest.c | 4 +-
tests/xml2vmxtest.c | 4 +-
tools/virsh.c | 4 +-
42 files changed, 148 insertions(+), 66 deletions(-)
--
1.8.1.4
8:41 a.m.
New subject: [libvirt] [PATCH 01/10] cleanup: Remove the duplicate header
Detected by a simple Shell script:
for i in $(find -type f -name "*.[ch]" | grep -v gnulib); do
awk 'BEGIN {
FS=" "
fail=0
}
/^# *include.*\.h[">]$/{
arr[$NF]++
}
END {
for (key in arr) {
if (arr[key] > 1) {
fail=1
printf("%d %s\n", arr[key], key)
}
}
if (fail == 1)
exit 1
}' $i
if test $? != 0; then
echo "Duplicate header(s) in $i"
fi
done;
A later patch will add the syntax-check to avoid duplicate
headers.
---
daemon/libvirtd.c | 1 -
src/conf/node_device_conf.c | 1 -
src/network/bridge_driver.c | 1 -
src/openvz/openvz_conf.c | 1 -
src/openvz/openvz_driver.c | 1 -
src/parallels/parallels_driver.c | 1 -
src/phyp/phyp_driver.c | 1 -
src/qemu/qemu_capabilities.h | 1 -
src/qemu/qemu_driver.c | 1 -
src/security/security_selinux.c | 1 -
src/uml/uml_driver.c | 1 -
src/util/virnetdevtap.c | 1 -
src/xen/xen_hypervisor.c | 1 -
13 files changed, 13 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 34a8737..38b7346 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -51,7 +51,6 @@
#include "virnetlink.h"
#include "virnetserver.h"
#include "remote.h"
-#include "remote_driver.h"
#include "virhook.h"
#include "viraudit.h"
#include "locking/lock_manager.h"
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index b4d8cb3..8fe4e03 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -31,7 +31,6 @@
#include "viralloc.h"
#include "node_device_conf.h"
-#include "viralloc.h"
#include "virxml.h"
#include "virutil.h"
#include "virbuffer.h"
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index e8b314a..3b27980 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -39,7 +39,6 @@
#include <signal.h>
#include <paths.h>
#include <pwd.h>
-#include <stdio.h>
#include <sys/wait.h>
#include <sys/ioctl.h>
#include <net/if.h>
diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 4b21c4e..a6f96c7 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -36,7 +36,6 @@
#include <dirent.h>
#include <time.h>
#include <sys/stat.h>
-#include <unistd.h>
#include <limits.h>
#include <errno.h>
#include <string.h>
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 9b10624..5d22c63 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -41,7 +41,6 @@
#include <fcntl.h>
#include <paths.h>
#include <pwd.h>
-#include <stdio.h>
#include <sys/wait.h>
#include "virerror.h"
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 5b8d85f..0c0bfcb 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -35,7 +35,6 @@
#include <fcntl.h>
#include <paths.h>
#include <pwd.h>
-#include <stdio.h>
#include <sys/wait.h>
#include <sys/time.h>
#include <sys/statvfs.h>
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 50e8216..3057345 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -33,7 +33,6 @@
#include <stdlib.h>
#include <unistd.h>
#include <errno.h>
-#include <stdio.h>
#include <libssh2.h>
#include <netinet/in.h>
#include <arpa/inet.h>
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 7a0f191..d903f6f 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -27,7 +27,6 @@
# include "virobject.h"
# include "capabilities.h"
# include "vircommand.h"
-# include "virobject.h"
# include "qemu_monitor.h"
/* Internal flags to keep track of qemu command line capabilities */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7c167b7..cee5557 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -38,7 +38,6 @@
#include <fcntl.h>
#include <signal.h>
#include <paths.h>
-#include <stdio.h>
#include <sys/wait.h>
#include <sys/ioctl.h>
#include <sys/un.h>
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 7333a1f..c620a2e 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -43,7 +43,6 @@
#include "virfile.h"
#include "virhash.h"
#include "virrandom.h"
-#include "virutil.h"
#include "virconf.h"
#include "virtpm.h"
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 4fc2315..498d1d9 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -38,7 +38,6 @@
#include <signal.h>
#include <paths.h>
#include <pwd.h>
-#include <stdio.h>
#include <sys/wait.h>
#include <sys/ioctl.h>
#include <sys/inotify.h>
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 871376e..e4ce223 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -29,7 +29,6 @@
#include "virnetdevopenvswitch.h"
#include "virerror.h"
#include "virfile.h"
-#include "virerror.h"
#include "viralloc.h"
#include "virlog.h"
#include "virutil.h"
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index e16fffe..9dbbe07 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -33,7 +33,6 @@
#include <sys/mman.h>
#include <sys/ioctl.h>
#include <limits.h>
-#include <stdint.h>
#include <regex.h>
#include <errno.h>
--
1.8.1.4
9:50 p.m.
New subject: [libvirt] [PATCH 01/10] cleanup: Remove the duplicate header
On 04/16/2013 07:41 AM, Osier Yang wrote:
Detected by a simple Shell script:
for i in $(find -type f -name "*.[ch]" | grep -v gnulib); do
You can limit things to version-controlled files a bit faster with:
for i in $(git ls-files -- '*.[ch]'); do
awk 'BEGIN {
FS=" "
fail=0
}
/^# *include.*\.h[">]$/{
This pattern misses files that include a header with a comment saying
why, such as this one:
src/util/virxml.c:#include <math.h> /* for isnan() */
arr[$NF]++
This doesn't catch duplication between "" and <> in the same file,
since
it includes the delimiter as part of the key name. Slightly more
powerful is:
/# *include/{
match($0, /["<][^">]*[">]/)
arr[substr($0, RSTART+1, RLENGTH-2)]++
}
which uses the array name of just the filename regardless of delimiters.
}
END {
for (key in arr) {
if (arr[key] > 1) {
fail=1
printf("%d %s\n", arr[key], key)
}
}
if (fail == 1)
exit 1
}' $i
if test $? != 0; then
echo "Duplicate header(s) in $i"
fi
done;
A later patch will add the syntax-check to avoid duplicate
headers.
---
What you have works, but with my improvements to the script, I still see
this after your patch:
2 testutils.h
Duplicate header(s) in ./tests/qemumonitortest.c
2 testutils.h
Duplicate header(s) in ./tests/qemuxmlnstest.c
2 testutils.h
Duplicate header(s) in ./tests/qemuxml2argvtest.c
2 testutils.h
Duplicate header(s) in ./tests/qemuargv2xmltest.c
2 testutils.h
Duplicate header(s) in ./tests/shunloadtest.c
2 testutils.h
Duplicate header(s) in ./tests/vmx2xmltest.c
2 testutils.h
Duplicate header(s) in ./tests/domainsnapshotxml2xmltest.c
2 testutils.h
Duplicate header(s) in ./tests/qemuhelptest.c
2 testutils.h
Duplicate header(s) in ./tests/lxcxml2xmltest.c
2 testutils.h
Duplicate header(s) in ./tests/xml2vmxtest.c
2 testutils.h
Duplicate header(s) in ./tests/qemuxml2xmltest.c
2 testutils.h
Duplicate header(s) in ./tests/esxutilstest.c
2 testutils.h
Duplicate header(s) in ./tests/openvzutilstest.c
2 libxl.h
Duplicate header(s) in ./src/libxl/libxl_driver.c
testutils.h is cleaned up in patch 2; but the duplicate libxl.h in
libxl_driver.c is still buggy.
ACK with libxl_driver.c also fixed (you can touch up the commit message
if you like, but that's not required; as long as we later get the syntax
check right).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:03 a.m.
New subject: [libvirt] [PATCH 01/10] cleanup: Remove the duplicate header
On 17/04/13 10:50, Eric Blake wrote:
On 04/16/2013 07:41 AM, Osier Yang wrote:
> Detected by a simple Shell script:
>
> for i in $(find -type f -name "*.[ch]" | grep -v gnulib); do
You can limit things to version-controlled files a bit faster with:
for i in $(git ls-files -- '*.[ch]'); do
Nice.
> awk 'BEGIN {
> FS=" "
> fail=0
> }
> /^# *include.*\.h[">]$/{
This pattern misses files that include a header with a comment saying
why, such as this one:
src/util/virxml.c:#include <math.h> /* for isnan() */
> arr[$NF]++
This doesn't catch duplication between "" and <> in the same file,
since
it includes the delimiter as part of the key name. Slightly more
powerful is:
/# *include/{
match($0, /["<][^">]*[">]/)
arr[substr($0, RSTART+1, RLENGTH-2)]++
}
Nice too. But it detects:
2 "virsh-edit.c" are included
Duplicate header(s) in virsh-domain.c
I changed it a bit to avoid it:
/# *include.*\.h/{
match($0, /["<][^">]*[">]/)
arr[substr($0, RSTART+1, RLENGTH-2)]++
}
Osier
8:41 a.m.
New subject: [libvirt] [PATCH 02/10] cleanup: Only include testutils.h once
No reason to include it in both "if" and "else" branches.
---
tests/domainsnapshotxml2xmltest.c | 4 ++--
tests/esxutilstest.c | 4 ++--
tests/lxcxml2xmltest.c | 4 ++--
tests/openvzutilstest.c | 4 ++--
tests/qemuargv2xmltest.c | 4 ++--
tests/qemuhelptest.c | 4 ++--
tests/qemumonitortest.c | 4 ++--
tests/qemuxml2argvtest.c | 4 ++--
tests/qemuxml2xmltest.c | 4 ++--
tests/qemuxmlnstest.c | 4 ++--
tests/shunloadtest.c | 4 ++--
tests/vmx2xmltest.c | 4 ++--
tests/xml2vmxtest.c | 4 ++--
13 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/tests/domainsnapshotxml2xmltest.c b/tests/domainsnapshotxml2xmltest.c
index 6f1c600..694c382 100644
--- a/tests/domainsnapshotxml2xmltest.c
+++ b/tests/domainsnapshotxml2xmltest.c
@@ -8,10 +8,11 @@
#include <sys/types.h>
#include <fcntl.h>
+#include "testutils.h"
+
#ifdef WITH_QEMU
# include "internal.h"
-# include "testutils.h"
# include "qemu/qemu_conf.h"
# include "qemu/qemu_domain.h"
# include "testutilsqemu.h"
@@ -125,7 +126,6 @@ mymain(void)
VIRT_TEST_MAIN(mymain)
#else
-# include "testutils.h"
int
main(void)
diff --git a/tests/esxutilstest.c b/tests/esxutilstest.c
index 55b3623..e2baea2 100644
--- a/tests/esxutilstest.c
+++ b/tests/esxutilstest.c
@@ -1,5 +1,7 @@
#include <config.h>
+#include "testutils.h"
+
#ifdef WITH_ESX
# include <stdio.h>
@@ -8,7 +10,6 @@
# include "internal.h"
# include "viralloc.h"
-# include "testutils.h"
# include "virutil.h"
# include "vmx/vmx.h"
# include "esx/esx_util.h"
@@ -279,7 +280,6 @@ mymain(void)
VIRT_TEST_MAIN(mymain)
#else
-# include "testutils.h"
int main(void)
{
diff --git a/tests/lxcxml2xmltest.c b/tests/lxcxml2xmltest.c
index e0f99f4..4870f42 100644
--- a/tests/lxcxml2xmltest.c
+++ b/tests/lxcxml2xmltest.c
@@ -8,10 +8,11 @@
#include <sys/types.h>
#include <fcntl.h>
+#include "testutils.h"
+
#ifdef WITH_LXC
# include "internal.h"
-# include "testutils.h"
# include "lxc/lxc_conf.h"
# include "testutilslxc.h"
@@ -136,7 +137,6 @@ mymain(void)
VIRT_TEST_MAIN(mymain)
#else
-# include "testutils.h"
int
main(void)
diff --git a/tests/openvzutilstest.c b/tests/openvzutilstest.c
index 9fb7178..616b6be 100644
--- a/tests/openvzutilstest.c
+++ b/tests/openvzutilstest.c
@@ -1,5 +1,7 @@
#include <config.h>
+#include "testutils.h"
+
#ifdef WITH_OPENVZ
# include <stdio.h>
@@ -8,7 +10,6 @@
# include "internal.h"
# include "viralloc.h"
-# include "testutils.h"
# include "virutil.h"
# include "openvz/openvz_conf.h"
@@ -160,7 +161,6 @@ mymain(void)
VIRT_TEST_MAIN(mymain)
#else
-# include "testutils.h"
int main(void)
{
diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
index 9167d88..ee6c7a9 100644
--- a/tests/qemuargv2xmltest.c
+++ b/tests/qemuargv2xmltest.c
@@ -8,10 +8,11 @@
#include <sys/types.h>
#include <fcntl.h>
+#include "testutils.h"
+
#ifdef WITH_QEMU
# include "internal.h"
-# include "testutils.h"
# include "qemu/qemu_command.h"
# include "testutilsqemu.h"
@@ -260,7 +261,6 @@ mymain(void)
VIRT_TEST_MAIN(mymain)
#else
-# include "testutils.h"
int
main(void)
diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
index 43774f4..7290183 100644
--- a/tests/qemuhelptest.c
+++ b/tests/qemuhelptest.c
@@ -1,11 +1,12 @@
#include <config.h>
+#include "testutils.h"
+
#ifdef WITH_QEMU
# include <stdio.h>
# include <stdlib.h>
-# include "testutils.h"
# include "qemu/qemu_capabilities.h"
# include "viralloc.h"
@@ -1034,7 +1035,6 @@ mymain(void)
VIRT_TEST_MAIN(mymain)
#else
-# include "testutils.h"
int main(void)
{
diff --git a/tests/qemumonitortest.c b/tests/qemumonitortest.c
index 285dfa8..a8fc57a 100644
--- a/tests/qemumonitortest.c
+++ b/tests/qemumonitortest.c
@@ -5,11 +5,12 @@
#include <string.h>
#include <unistd.h>
+#include "testutils.h"
+
#ifdef WITH_QEMU
# include "internal.h"
# include "viralloc.h"
-# include "testutils.h"
# include "virutil.h"
# include "qemu/qemu_monitor.h"
@@ -108,7 +109,6 @@ mymain(void)
VIRT_TEST_MAIN(mymain)
#else
-# include "testutils.h"
int main(void)
{
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index be6c759..4bf13f0 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -8,10 +8,11 @@
#include <sys/types.h>
#include <fcntl.h>
+#include "testutils.h"
+
#ifdef WITH_QEMU
# include "internal.h"
-# include "testutils.h"
# include "viralloc.h"
# include "qemu/qemu_capabilities.h"
# include "qemu/qemu_command.h"
@@ -951,7 +952,6 @@ mymain(void)
VIRT_TEST_MAIN(mymain)
#else
-# include "testutils.h"
int main(void)
{
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 3a1683f..7434190 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -8,10 +8,11 @@
#include <sys/types.h>
#include <fcntl.h>
+#include "testutils.h"
+
#ifdef WITH_QEMU
# include "internal.h"
-# include "testutils.h"
# include "qemu/qemu_conf.h"
# include "qemu/qemu_domain.h"
# include "testutilsqemu.h"
@@ -285,7 +286,6 @@ mymain(void)
VIRT_TEST_MAIN(mymain)
#else
-# include "testutils.h"
int
main(void)
diff --git a/tests/qemuxmlnstest.c b/tests/qemuxmlnstest.c
index e6b42dc..4e9144b 100644
--- a/tests/qemuxmlnstest.c
+++ b/tests/qemuxmlnstest.c
@@ -8,10 +8,11 @@
#include <sys/types.h>
#include <fcntl.h>
+#include "testutils.h"
+
#ifdef WITH_QEMU
# include "internal.h"
-# include "testutils.h"
# include "qemu/qemu_capabilities.h"
# include "qemu/qemu_command.h"
# include "qemu/qemu_domain.h"
@@ -263,7 +264,6 @@ mymain(void)
VIRT_TEST_MAIN(mymain)
#else
-# include "testutils.h"
int main(void)
{
diff --git a/tests/shunloadtest.c b/tests/shunloadtest.c
index eddb818..8271b93 100644
--- a/tests/shunloadtest.c
+++ b/tests/shunloadtest.c
@@ -40,6 +40,8 @@
#include <config.h>
+#include "testutils.h"
+
#ifdef linux
# include <dlfcn.h>
@@ -50,7 +52,6 @@
# include <signal.h>
# include "internal.h"
-# include "testutils.h"
pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
@@ -143,7 +144,6 @@ int main(int argc ATTRIBUTE_UNUSED, char **argv)
}
#else
-# include "testutils.h"
int main(void)
{
diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c
index 2d04da7..0003f5f 100644
--- a/tests/vmx2xmltest.c
+++ b/tests/vmx2xmltest.c
@@ -1,5 +1,7 @@
#include <config.h>
+#include "testutils.h"
+
#ifdef WITH_VMX
# include <stdio.h>
@@ -8,7 +10,6 @@
# include "internal.h"
# include "viralloc.h"
-# include "testutils.h"
# include "vmx/vmx.h"
static virCapsPtr caps;
@@ -298,7 +299,6 @@ mymain(void)
VIRT_TEST_MAIN(mymain)
#else
-# include "testutils.h"
int main(void)
{
diff --git a/tests/xml2vmxtest.c b/tests/xml2vmxtest.c
index 1957659..fa4f6f9 100644
--- a/tests/xml2vmxtest.c
+++ b/tests/xml2vmxtest.c
@@ -1,5 +1,7 @@
#include <config.h>
+#include "testutils.h"
+
#ifdef WITH_VMX
# include <stdio.h>
@@ -8,7 +10,6 @@
# include "internal.h"
# include "viralloc.h"
-# include "testutils.h"
# include "vmx/vmx.h"
static virCapsPtr caps;
@@ -311,7 +312,6 @@ mymain(void)
VIRT_TEST_MAIN(mymain)
#else
-# include "testutils.h"
int main(void)
{
--
1.8.1.4
9:53 p.m.
New subject: [libvirt] [PATCH 02/10] cleanup: Only include testutils.h once
On 04/16/2013 07:41 AM, Osier Yang wrote:
No reason to include it in both "if" and "else"
branches.
---
tests/domainsnapshotxml2xmltest.c | 4 ++--
tests/esxutilstest.c | 4 ++--
tests/lxcxml2xmltest.c | 4 ++--
tests/openvzutilstest.c | 4 ++--
tests/qemuargv2xmltest.c | 4 ++--
tests/qemuhelptest.c | 4 ++--
tests/qemumonitortest.c | 4 ++--
tests/qemuxml2argvtest.c | 4 ++--
tests/qemuxml2xmltest.c | 4 ++--
tests/qemuxmlnstest.c | 4 ++--
tests/shunloadtest.c | 4 ++--
tests/vmx2xmltest.c | 4 ++--
tests/xml2vmxtest.c | 4 ++--
13 files changed, 26 insertions(+), 26 deletions(-)
ACK. If you rebase this one before 1/10, then the script in 1/10 (after
my touchups and a final cleanup of libxl) has no hits.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:41 a.m.
New subject: [libvirt] [PATCH 03/10] cleanup: Don't include libvirt/libvirt.h
Which is already included by "internal.h", later patch will add
syntax-check to avoid it.
---
src/nodeinfo.h | 1 -
src/phyp/phyp_driver.c | 1 -
src/remote/remote_protocol.x | 1 -
src/util/virkeycode.h | 1 -
4 files changed, 4 deletions(-)
diff --git a/src/nodeinfo.h b/src/nodeinfo.h
index 350f3c3..b0e5fbc 100644
--- a/src/nodeinfo.h
+++ b/src/nodeinfo.h
@@ -24,7 +24,6 @@
#ifndef __VIR_NODEINFO_H__
# define __VIR_NODEINFO_H__
-# include "libvirt/libvirt.h"
# include "capabilities.h"
int nodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo);
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 3057345..d6d9b0c 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -49,7 +49,6 @@
#include "viralloc.h"
#include "virlog.h"
#include "driver.h"
-#include "libvirt/libvirt.h"
#include "virerror.h"
#include "viruuid.h"
#include "domain_conf.h"
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index b957b8e..d384225 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -36,7 +36,6 @@
* 'REMOTE_'. This makes names quite long.
*/
-%#include <libvirt/libvirt.h>
%#include "internal.h"
%#include <arpa/inet.h>
diff --git a/src/util/virkeycode.h b/src/util/virkeycode.h
index a2e1391..6947cfe 100644
--- a/src/util/virkeycode.h
+++ b/src/util/virkeycode.h
@@ -23,7 +23,6 @@
# define __VIR_UTIL_VIRTKEYCODE_H__
# include "virutil.h"
-# include "libvirt/libvirt.h"
VIR_ENUM_DECL(virKeycodeSet);
int virKeycodeValueFromString(virKeycodeSet codeset, const char *keyname);
--
1.8.1.4
10:01 p.m.
New subject: [libvirt] [PATCH 03/10] cleanup: Don't include libvirt/libvirt.h
On 04/16/2013 07:41 AM, Osier Yang wrote:
Which is already included by "internal.h", later patch will
add
syntax-check to avoid it.
---
ACK
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:20 p.m.
New subject: [libvirt] [PATCH 03/10] cleanup: Don't include libvirt/libvirt.h
On 04/16/2013 07:41 AM, Osier Yang wrote:
Which is already included by "internal.h", later patch will
add
syntax-check to avoid it.
---
+++ b/src/nodeinfo.h
@@ -24,7 +24,6 @@
#ifndef __VIR_NODEINFO_H__
# define __VIR_NODEINFO_H__
-# include "libvirt/libvirt.h"
# include "capabilities.h"
I took a second look at this after 4/10; thankfully, here you are still
self-contained, via indirection, since capabilities.h includes internal.h.
int nodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo);
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 3057345..d6d9b0c 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -49,7 +49,6 @@
#include "viralloc.h"
#include "virlog.h"
#include "driver.h"
-#include "libvirt/libvirt.h"
This is clean (internal.h is several lines earlier)
#include "virerror.h"
#include "viruuid.h"
#include "domain_conf.h"
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index b957b8e..d384225 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -36,7 +36,6 @@
* 'REMOTE_'. This makes names quite long.
*/
-%#include <libvirt/libvirt.h>
%#include "internal.h"
This is clean.
%#include <arpa/inet.h>
diff --git a/src/util/virkeycode.h b/src/util/virkeycode.h
index a2e1391..6947cfe 100644
--- a/src/util/virkeycode.h
+++ b/src/util/virkeycode.h
@@ -23,7 +23,6 @@
# define __VIR_UTIL_VIRTKEYCODE_H__
# include "virutil.h"
-# include "libvirt/libvirt.h"
This relies on indirect inclusion via virutil.h. We've used indirect
inclusion before (in fact, internal.h lets us indirectly include common
headers like <errno.h> instead of repeating it everywhere), so that's
fine (I know some other projects like all .h files to be self-contained
without indirect inclusion, but that's a style choice; I'm not proposing
that libvirt adopt it).
So my ACK still stands.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
5:58 a.m.
New subject: [libvirt] [PATCH 03/10] cleanup: Don't include libvirt/libvirt.h
On Tue, Apr 16, 2013 at 09:41:45PM +0800, Osier Yang wrote:
Which is already included by "internal.h", later patch will
add
syntax-check to avoid it.
---
src/nodeinfo.h | 1 -
src/phyp/phyp_driver.c | 1 -
src/remote/remote_protocol.x | 1 -
src/util/virkeycode.h | 1 -
4 files changed, 4 deletions(-)
diff --git a/src/remote/remote_protocol.x
b/src/remote/remote_protocol.x
index b957b8e..d384225 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -36,7 +36,6 @@
* 'REMOTE_'. This makes names quite long.
*/
-%#include <libvirt/libvirt.h>
%#include "internal.h"
%#include <arpa/inet.h>
This has broken make check
GEN remote_protocol-struct
--- remote_protocol-structs 2013-04-11 18:02:02.445662659 +0100
+++ remote_protocol-struct-t3 2013-04-17 11:57:29.472083827 +0100
@@ -7,6 +7,7 @@
VIR_TYPED_PARAM_DOUBLE = 5,
VIR_TYPED_PARAM_BOOLEAN = 6,
VIR_TYPED_PARAM_STRING = 7,
+ VIR_TYPED_PARAM_LAST = 8,
};
struct remote_nonnull_domain {
remote_nonnull_string name;
make[2]: *** [remote_protocol-struct] Error 1
The reason is that previously the remote driver would not see the
sentinel values from enums, but now it does.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
7:38 a.m.
New subject: [libvirt] [PATCH 03/10] cleanup: Don't include libvirt/libvirt.h
On 17/04/13 18:58, Daniel P. Berrange wrote:
On Tue, Apr 16, 2013 at 09:41:45PM +0800, Osier Yang wrote:
> Which is already included by "internal.h", later patch will add
> syntax-check to avoid it.
> ---
> src/nodeinfo.h | 1 -
> src/phyp/phyp_driver.c | 1 -
> src/remote/remote_protocol.x | 1 -
> src/util/virkeycode.h | 1 -
> 4 files changed, 4 deletions(-)
> diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
> index b957b8e..d384225 100644
> --- a/src/remote/remote_protocol.x
> +++ b/src/remote/remote_protocol.x
> @@ -36,7 +36,6 @@
> * 'REMOTE_'. This makes names quite long.
> */
>
> -%#include <libvirt/libvirt.h>
> %#include "internal.h"
> %#include <arpa/inet.h>
>
This has broken make check
GEN remote_protocol-struct
--- remote_protocol-structs 2013-04-11 18:02:02.445662659 +0100
+++ remote_protocol-struct-t3 2013-04-17 11:57:29.472083827 +0100
@@ -7,6 +7,7 @@
VIR_TYPED_PARAM_DOUBLE = 5,
VIR_TYPED_PARAM_BOOLEAN = 6,
VIR_TYPED_PARAM_STRING = 7,
+ VIR_TYPED_PARAM_LAST = 8,
};
struct remote_nonnull_domain {
remote_nonnull_string name;
make[2]: *** [remote_protocol-struct] Error 1
I'm wondering why it works on my FC18.
Osier
10:07 a.m.
New subject: [libvirt] [PATCH 03/10] cleanup: Don't include libvirt/libvirt.h
On 04/17/2013 06:38 AM, Osier Yang wrote:
>> */
>> -%#include <libvirt/libvirt.h>
>> %#include "internal.h"
>> %#include <arpa/inet.h>
>>
> This has broken make check
Let's revert this hunk, and stop enforcing the checks on .x files, then.
>
> GEN remote_protocol-struct
> --- remote_protocol-structs 2013-04-11 18:02:02.445662659 +0100
> +++ remote_protocol-struct-t3 2013-04-17 11:57:29.472083827 +0100
> @@ -7,6 +7,7 @@
> VIR_TYPED_PARAM_DOUBLE = 5,
> VIR_TYPED_PARAM_BOOLEAN = 6,
> VIR_TYPED_PARAM_STRING = 7,
> + VIR_TYPED_PARAM_LAST = 8,
We _don't_ want the wire protocol to be exposing *_LAST enum elements.
That's _why_ we included <libvirt/libvirt.h> first (to get only public
names), then "internal.h" second (to get everything else). Including
"internal.h" first (or in isolation) turns on private names such as
*_LAST elements. I forgot about that, and hadn't actually tested your
patch (I was just reviewing it based on content), or I would have
spotted it sooner.
> };
> struct remote_nonnull_domain {
> remote_nonnull_string name;
> make[2]: *** [remote_protocol-struct] Error 1
>
>
I'm wondering why it works on my FC18.
Probably because you don't have dwarves installed, or if you do, because
there have been known-buggy versions of dwarves where pdwtags fails to
parse files compiled by newer gcc versions; in either of those
situations, 'make check' no longer validates the remote_protocol-struct
file.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:24 a.m.
New subject: [libvirt] [PATCH 03/10] cleanup: Don't include libvirt/libvirt.h
On 17/04/13 23:07, Eric Blake wrote:
On 04/17/2013 06:38 AM, Osier Yang wrote:
>>> */
>>> -%#include <libvirt/libvirt.h>
>>> %#include "internal.h"
>>> %#include <arpa/inet.h>
>>>
>> This has broken make check
Let's revert this hunk, and stop enforcing the checks on .x files, then.
>> GEN remote_protocol-struct
>> --- remote_protocol-structs 2013-04-11 18:02:02.445662659 +0100
>> +++ remote_protocol-struct-t3 2013-04-17 11:57:29.472083827 +0100
>> @@ -7,6 +7,7 @@
>> VIR_TYPED_PARAM_DOUBLE = 5,
>> VIR_TYPED_PARAM_BOOLEAN = 6,
>> VIR_TYPED_PARAM_STRING = 7,
>> + VIR_TYPED_PARAM_LAST = 8,
We _don't_ want the wire protocol to be exposing *_LAST enum elements.
That's _why_ we included <libvirt/libvirt.h> first (to get only public
names), then "internal.h" second (to get everything else). Including
"internal.h" first (or in isolation) turns on private names such as
*_LAST elements. I forgot about that, and hadn't actually tested your
patch (I was just reviewing it based on content), or I would have
spotted it sooner.
Hum, I see.
>> };
>> struct remote_nonnull_domain {
>> remote_nonnull_string name;
>> make[2]: *** [remote_protocol-struct] Error 1
>>
>>
> I'm wondering why it works on my FC18.
Probably because you don't have dwarves installed, or if you do, because
there have been known-buggy versions of dwarves where pdwtags fails to
parse files compiled by newer gcc versions; in either of those
situations, 'make check' no longer validates the remote_protocol-struct
file.
Right, I have not installed it, :(
Pushed the reverting patch in build-breaker rule:
commit f043199413c8263a17802cf2c0636bc1fe348772
Author: Osier Yang <jyang(a)redhat.com>
Date: Wed Apr 17 23:14:52 2013 +0800
remote: Revert removing "libvirt/libvirt.h" in remote_protocol.x
Commit 2d25fd4f410f removed the including of "libvirt/libvirt.h",
which breaks the build. Pushed under build-breaker rule.
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index d384225..b957b8e 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -36,6 +36,7 @@
* 'REMOTE_'. This makes names quite long.
*/
+%#include <libvirt/libvirt.h>
%#include "internal.h"
%#include <arpa/inet.h>
Osier
8:41 a.m.
New subject: [libvirt] [PATCH 04/10] cleanup: Don't include libvirt/virterror.h
Which is already included in "internal.h", later patch will add
syntax-check to avoid it.
---
src/remote/remote_driver.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/remote/remote_driver.h b/src/remote/remote_driver.h
index 7288381..bd907c0 100644
--- a/src/remote/remote_driver.h
+++ b/src/remote/remote_driver.h
@@ -24,8 +24,6 @@
#ifndef __VIR_REMOTE_INTERNAL_H__
# define __VIR_REMOTE_INTERNAL_H__
-# include "libvirt/virterror.h"
-
# include "configmake.h"
int remoteRegister (void);
--
1.8.1.4
10:16 p.m.
New subject: [libvirt] [PATCH 04/10] cleanup: Don't include libvirt/virterror.h
On 04/16/2013 07:41 AM, Osier Yang wrote:
Which is already included in "internal.h", later patch will
add
syntax-check to avoid it.
---
src/remote/remote_driver.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/remote/remote_driver.h b/src/remote/remote_driver.h
index 7288381..bd907c0 100644
--- a/src/remote/remote_driver.h
+++ b/src/remote/remote_driver.h
@@ -24,8 +24,6 @@
#ifndef __VIR_REMOTE_INTERNAL_H__
# define __VIR_REMOTE_INTERNAL_H__
-# include "libvirt/virterror.h"
-
# include "configmake.h"
Except that none of the remaining includes pull in internal.h, so this
change would make the header not self-contained. I'd be happier if you
did s#libvirt/virterror.h#internal.h# instead of deleting the line.
ACK with that change.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:01 a.m.
New subject: [libvirt] [PATCH 04/10] cleanup: Don't include libvirt/virterror.h
On 17/04/13 11:16, Eric Blake wrote:
On 04/16/2013 07:41 AM, Osier Yang wrote:
> Which is already included in "internal.h", later patch will add
> syntax-check to avoid it.
> ---
> src/remote/remote_driver.h | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/src/remote/remote_driver.h b/src/remote/remote_driver.h
> index 7288381..bd907c0 100644
> --- a/src/remote/remote_driver.h
> +++ b/src/remote/remote_driver.h
> @@ -24,8 +24,6 @@
> #ifndef __VIR_REMOTE_INTERNAL_H__
> # define __VIR_REMOTE_INTERNAL_H__
>
> -# include "libvirt/virterror.h"
> -
> # include "configmake.h"
Except that none of the remaining includes pull in internal.h, so this
change would make the header not self-contained. I'd be happier if you
did s#libvirt/virterror.h#internal.h# instead of deleting the line.
ACK with that change.
I pushed the 1/4 ~ 4/4. with this change; the shell script improved with
your
suggestion in 1/4's commit log; 2/4 is rebased before 1/4. Thanks.
Osier
8:41 a.m.
New subject: [libvirt] [PATCH 05/10] syntax-check: Don't include duplicate header
Except gnulib...
---
cfg.mk | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/cfg.mk b/cfg.mk
index e60c4e3..71f7ee4 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -722,6 +722,28 @@ sc_prohibit_exit_in_tests:
halt='use return, not exit(), in tests' \
$(_sc_search_regexp)
+# Don't include duplicate header in the source (either *.c or *.h)
+sc_prohibit_duplicate_header:
+ @for i in $$($(VC_LIST_EXCEPT) | grep '\.[ch]$$'); do \
+ awk 'BEGIN { \
+ FS=" "; \
+ fail=0; \
+ } \
+ /^# *include.*\.h[">]$$/ { \
+ arr[$$NF]++; \
+ } \
+ END { \
+ for (key in arr) { \
+ if (arr[key] > 1) { \
+ fail=1; \
+ printf("%d %s are included\n", arr[key], key); \
+ } \
+ } \
+ if (fail == 1) \
+ exit 1; \
+ }' $$i || { echo "Duplicate header(s) in $$i"; exit 1; }; \
+ done
+
# We don't use this feature of maint.mk.
prev_version_file = /dev/null
--
1.8.1.4
10:41 p.m.
New subject: [libvirt] [PATCH 05/10] syntax-check: Don't include duplicate header
On 04/16/2013 07:41 AM, Osier Yang wrote:
Except gnulib...
---
cfg.mk | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/cfg.mk b/cfg.mk
index e60c4e3..71f7ee4 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -722,6 +722,28 @@ sc_prohibit_exit_in_tests:
halt='use return, not exit(), in tests' \
$(_sc_search_regexp)
+# Don't include duplicate header in the source (either *.c or *.h)
+sc_prohibit_duplicate_header:
+ @for i in $$($(VC_LIST_EXCEPT) | grep '\.[ch]$$'); do \
You also cleaned up .x files, so use [chx] instead of [ch].
+ awk 'BEGIN { \
+ FS=" "; \
I don't think you need to set FS, if you use my match/substr change.
+ fail=0; \
+ } \
+ /^# *include.*\.h[">]$$/ { \
+ arr[$$NF]++; \
+ } \
Here's where my comments on patch 1 should be incorporated:
/# *include/ { \
match($0, /[<"][^>"]*[">]/) \
arr[substr($0, RMATCH + 1, RLENGTH - 2)]++ \
}
+ END { \
+ for (key in arr) { \
+ if (arr[key] > 1) { \
+ fail=1; \
+ printf("%d %s are included\n", arr[key], key); \
+ } \
+ } \
+ if (fail == 1) \
+ exit 1; \
+ }' $$i || { echo "Duplicate header(s) in $$i"; exit 1; }; \
This exits on first failure, instead of collecting all failures in one
go. It also misses the prefix $(ME): used in other error messages, and
should be sent to stderr. I would do:
@fail=0; for i in ...
awk '{
...
if (fail == 1) {
printf("duplicate header(s) in " FILENAME
}
}' $$i || fail=1
done; if test $$fail = 1; then
{ echo "$(ME): avoid duplicate headers" >&2; exit 1; }
fi
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:41 a.m.
New subject: [libvirt] [PATCH 06/10] syntax-check: Don't include "libvirt.h" and "virterror.h" in "" form
Directories python/tools/examples should include them in <> form,
though this patch allows "" form in these directories by excluding
them, a later patch will do the cleanup.
---
cfg.mk | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/cfg.mk b/cfg.mk
index 71f7ee4..cb8079c 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -744,6 +744,20 @@ sc_prohibit_duplicate_header:
}' $$i || { echo "Duplicate header(s) in $$i"; exit 1; }; \
done
+# Don't include "libvirt/libvirt.h" in "" form.
+sc_prohibit_include_libvirt_h:
+ @prohibit='^# *include *"libvirt/libvirt\.h"' \
+ in_vc_files='\.[ch]$$' \
+ halt='Do not include libvirt/libvirt.h' \
+ $(_sc_search_regexp)
+
+# Don't include "libvirt/virterror.h" in "" form.
+sc_prohibit_include_virterror_h:
+ @prohibit='^# *include *"libvirt/virterror\.h"' \
+ in_vc_files='\.[ch]$$' \
+ halt='Do not include libvirt/virterror.h' \
+ $(_sc_search_regexp)
+
# We don't use this feature of maint.mk.
prev_version_file = /dev/null
@@ -897,3 +911,9 @@ exclude_file_name_regexp--sc_correct_id_types = \
(^src/locking/lock_protocol.x$$)
exclude_file_name_regexp--sc_m4_quote_check = m4/virt-lib.m4
+
+exclude_file_name_regexp--sc_prohibit_include_libvirt_h = \
+
^(src/internal\.h)|(include/libvirt/libvirt-(lxc|qemu)\.h)|(python/libvirt-override\.c)|(python/typewrappers\.h)$$
+
+exclude_file_name_regexp--sc_prohibit_include_virterror_h = \
+ ^(src/internal\.h)|(python/libvirt-|(lxc|qemu)-override\.c)|(python/typewrappers\.h)$$
--
1.8.1.4
10:51 p.m.
New subject: [libvirt] [PATCH 06/10] syntax-check: Don't include "libvirt.h" and "virterror.h" in "" form
On 04/16/2013 07:41 AM, Osier Yang wrote:
Directories python/tools/examples should include them in <>
form,
though this patch allows "" form in these directories by excluding
them, a later patch will do the cleanup.
---
cfg.mk | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
+# Don't include "libvirt/libvirt.h" in ""
form.
+sc_prohibit_include_libvirt_h:
+ @prohibit='^# *include *"libvirt/libvirt\.h"' \
+ in_vc_files='\.[ch]$$' \
+ halt='Do not include libvirt/libvirt.h' \
+ $(_sc_search_regexp)
+
+# Don't include "libvirt/virterror.h" in "" form.
+sc_prohibit_include_virterror_h:
+ @prohibit='^# *include *"libvirt/virterror\.h"' \
Combine these into one rule, for faster execution of 'make
syntax-check'. Also, while we generally use #include with # in column
1, C allows it in later columns and a stronger test avoids an anchor
(but if that opens up false negatives, then adding the anchor back in is
fine).
@prohibit='# *include *"libvirt/*\.h"'
+ in_vc_files='\.[ch]$$' \
Another case where .[chx] might be better, since we cleaned up .x files.
+ halt='Do not include libvirt/virterror.h' \
This halt message needs alteration when you merge the two checks into one.
+
+exclude_file_name_regexp--sc_prohibit_include_libvirt_h = \
+
^(src/internal\.h)|(include/libvirt/libvirt-(lxc|qemu)\.h)|(python/libvirt-override\.c)|(python/typewrappers\.h)$$
+
+exclude_file_name_regexp--sc_prohibit_include_virterror_h = \
+ ^(src/internal\.h)|(python/libvirt-|(lxc|qemu)-override\.c)|(python/typewrappers\.h)$$
and if you merge the rules, come up with a common name, and then you
only need one exclude rule. If we are excluding entire directories, we
could write this more compact as:
^(src/internal\.h$$\|python/\|tools/\|examples/\|include/libvirt/libvirt-*\.h$$)
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:41 a.m.
New subject: [libvirt] [PATCH 07/10] syntax-check: Include libvirt.h and virterror.h in <> form only in external tools
With this patch, include "libvirt.h" and "virterror.h" in ""
form
is only allowed for "internal.h". And only the external tools
(examples|tools|python|include/libvirt) can include the two headers
in <> form.
---
cfg.mk | 30 ++++++++++++++++++++++++++----
include/libvirt/libvirt-lxc.h | 2 +-
include/libvirt/libvirt-qemu.h | 2 +-
python/libvirt-lxc-override.c | 4 ++--
python/libvirt-override.c | 4 ++--
python/libvirt-qemu-override.c | 4 ++--
python/typewrappers.h | 4 ++--
tests/shunloadhelper.c | 2 --
8 files changed, 36 insertions(+), 16 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index cb8079c..98c7e40 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -748,14 +748,30 @@ sc_prohibit_duplicate_header:
sc_prohibit_include_libvirt_h:
@prohibit='^# *include *"libvirt/libvirt\.h"' \
in_vc_files='\.[ch]$$' \
- halt='Do not include libvirt/libvirt.h' \
+ halt='Do not include libvirt/libvirt.h in internal source' \
$(_sc_search_regexp)
# Don't include "libvirt/virterror.h" in "" form.
sc_prohibit_include_virterror_h:
@prohibit='^# *include *"libvirt/virterror\.h"' \
in_vc_files='\.[ch]$$' \
- halt='Do not include libvirt/virterror.h' \
+ halt='Do not include libvirt/virterror.h in internal source' \
+ $(_sc_search_regexp)
+
+# Don't include "libvirt/libvirt.h" in <> form. Except external
tools, e.g.
+# python binding, examples and tools subdirectories.
+sc_prohibit_include_libvirt_h_1:
+ @prohibit='^# *include *<libvirt/libvirt\.h>' \
+ in_vc_files='\.[ch]$$' \
+ halt='Do not include libvirt/libvirt.h in internal source' \
+ $(_sc_search_regexp)
+
+# Don't include "libvirt/virterror.h" in <> form. Except external
tools, e.g.
+# python binding, examples and tools subdirectories.
+sc_prohibit_include_virterror_h_1:
+ @prohibit='^# *include *<libvirt/virterror\.h>' \
+ in_vc_files='\.[ch]$$' \
+ halt='Do not include libvirt/virterror.h in internal source' \
$(_sc_search_regexp)
# We don't use this feature of maint.mk.
@@ -913,7 +929,13 @@ exclude_file_name_regexp--sc_correct_id_types = \
exclude_file_name_regexp--sc_m4_quote_check = m4/virt-lib.m4
exclude_file_name_regexp--sc_prohibit_include_libvirt_h = \
-
^(src/internal\.h)|(include/libvirt/libvirt-(lxc|qemu)\.h)|(python/libvirt-override\.c)|(python/typewrappers\.h)$$
+ ^src/internal\.h$$
exclude_file_name_regexp--sc_prohibit_include_virterror_h = \
- ^(src/internal\.h)|(python/libvirt-|(lxc|qemu)-override\.c)|(python/typewrappers\.h)$$
+ ^src/internal\.h$$
+
+exclude_file_name_regexp--sc_prohibit_include_libvirt_h_1 = \
+ ^(examples/|tools/|python/|include/libvirt/)
+
+exclude_file_name_regexp--sc_prohibit_include_virterror_h_1 = \
+ ^(examples/|tools/|python/|include/libvirt/)
diff --git a/include/libvirt/libvirt-lxc.h b/include/libvirt/libvirt-lxc.h
index 5021813..1901fce 100644
--- a/include/libvirt/libvirt-lxc.h
+++ b/include/libvirt/libvirt-lxc.h
@@ -26,7 +26,7 @@
#ifndef __VIR_LXC_H__
# define __VIR_LXC_H__
-# include "libvirt/libvirt.h"
+# include <libvirt/libvirt.h>
# ifdef __cplusplus
extern "C" {
diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h
index 8ec12b4..3e79a8c 100644
--- a/include/libvirt/libvirt-qemu.h
+++ b/include/libvirt/libvirt-qemu.h
@@ -26,7 +26,7 @@
#ifndef __VIR_QEMU_H__
# define __VIR_QEMU_H__
-# include "libvirt/libvirt.h"
+# include <libvirt/libvirt.h>
# ifdef __cplusplus
extern "C" {
diff --git a/python/libvirt-lxc-override.c b/python/libvirt-lxc-override.c
index c80668e..ead175f 100644
--- a/python/libvirt-lxc-override.c
+++ b/python/libvirt-lxc-override.c
@@ -17,8 +17,8 @@
#undef HAVE_PTHREAD_H
#include <Python.h>
-#include "libvirt/libvirt-lxc.h"
-#include "libvirt/virterror.h"
+#include <libvirt/libvirt-lxc.h>
+#include <libvirt/virterror.h>
#include "typewrappers.h"
#include "libvirt-lxc.h"
#include "viralloc.h"
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index f6573e1..3d8490c 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -20,8 +20,8 @@
#define VIR_ENUM_SENTINELS
#include <Python.h>
-#include "libvirt/libvirt.h"
-#include "libvirt/virterror.h"
+#include <libvirt/libvirt.h>
+#include <libvirt/virterror.h>
#include "typewrappers.h"
#include "libvirt.h"
#include "viralloc.h"
diff --git a/python/libvirt-qemu-override.c b/python/libvirt-qemu-override.c
index 243692a..8f1ce5e 100644
--- a/python/libvirt-qemu-override.c
+++ b/python/libvirt-qemu-override.c
@@ -17,8 +17,8 @@
#undef HAVE_PTHREAD_H
#include <Python.h>
-#include "libvirt/libvirt-qemu.h"
-#include "libvirt/virterror.h"
+#include <libvirt/libvirt-qemu.h>
+#include <libvirt/virterror.h>
#include "typewrappers.h"
#include "libvirt-qemu.h"
diff --git a/python/typewrappers.h b/python/typewrappers.h
index af68bce..d871d3f 100644
--- a/python/typewrappers.h
+++ b/python/typewrappers.h
@@ -8,8 +8,8 @@
#include <Python.h>
#include <stdbool.h>
-#include "libvirt/libvirt.h"
-#include "libvirt/virterror.h"
+#include <libvirt/libvirt.h>
+#include <libvirt/virterror.h>
#ifdef __GNUC__
# ifdef ATTRIBUTE_UNUSED
diff --git a/tests/shunloadhelper.c b/tests/shunloadhelper.c
index 1b025ee..a7bc2cc 100644
--- a/tests/shunloadhelper.c
+++ b/tests/shunloadhelper.c
@@ -28,8 +28,6 @@
#include <config.h>
#include "internal.h"
-#include <libvirt/libvirt.h>
-#include <libvirt/virterror.h>
#include <stdlib.h>
static void shunloadError(void *userData ATTRIBUTE_UNUSED,
--
1.8.1.4
11:03 p.m.
New subject: [libvirt] [PATCH 07/10] syntax-check: Include libvirt.h and virterror.h in <> form only in external tools
On 04/16/2013 07:41 AM, Osier Yang wrote:
With this patch, include "libvirt.h" and
"virterror.h" in "" form
is only allowed for "internal.h". And only the external tools
(examples|tools|python|include/libvirt) can include the two headers
in <> form.
---
Hmm. It sounds like you want two syntax checks after all, in order to
allow <> but not "" in the subdirectories; but it can still be done in
two rules instead of four.
cfg.mk | 30 ++++++++++++++++++++++++++----
include/libvirt/libvirt-lxc.h | 2 +-
include/libvirt/libvirt-qemu.h | 2 +-
python/libvirt-lxc-override.c | 4 ++--
python/libvirt-override.c | 4 ++--
python/libvirt-qemu-override.c | 4 ++--
python/typewrappers.h | 4 ++--
tests/shunloadhelper.c | 2 --
8 files changed, 36 insertions(+), 16 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index cb8079c..98c7e40 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -748,14 +748,30 @@ sc_prohibit_duplicate_header:
sc_prohibit_include_libvirt_h:
I'd name this sc_prohibit_include_libvirt_quote (that affects patch 6);
see below for why...
@prohibit='^# *include
*"libvirt/libvirt\.h"' \
in_vc_files='\.[ch]$$' \
- halt='Do not include libvirt/libvirt.h' \
+ halt='Do not include libvirt/libvirt.h in internal source' \
Squash this wording into patch 6.
$(_sc_search_regexp)
# Don't include "libvirt/virterror.h" in "" form.
sc_prohibit_include_virterror_h:
@prohibit='^# *include *"libvirt/virterror\.h"' \
in_vc_files='\.[ch]$$' \
- halt='Do not include libvirt/virterror.h' \
+ halt='Do not include libvirt/virterror.h in internal source' \
+ $(_sc_search_regexp)
+
+# Don't include "libvirt/libvirt.h" in <> form. Except external
tools, e.g.
+# python binding, examples and tools subdirectories.
+sc_prohibit_include_libvirt_h_1:
...here's why. The _1 suffix doesn't describe anything. So I'd name
this sc_prohibit_include_libvirt_brackets
+ @prohibit='^# *include *<libvirt/libvirt\.h>' \
+ in_vc_files='\.[ch]$$' \
+ halt='Do not include libvirt/libvirt.h in internal source' \
+ $(_sc_search_regexp)
+
+# Don't include "libvirt/virterror.h" in <> form. Except external
tools, e.g.
+# python binding, examples and tools subdirectories.
+sc_prohibit_include_virterror_h_1:
+ @prohibit='^# *include *<libvirt/virterror\.h>' \
+ in_vc_files='\.[ch]$$' \
+ halt='Do not include libvirt/virterror.h in internal source' \
$(_sc_search_regexp)
Again, these two can be merged into one.
# We don't use this feature of maint.mk.
@@ -913,7 +929,13 @@ exclude_file_name_regexp--sc_correct_id_types = \
exclude_file_name_regexp--sc_m4_quote_check = m4/virt-lib.m4
exclude_file_name_regexp--sc_prohibit_include_libvirt_h = \
-
^(src/internal\.h)|(include/libvirt/libvirt-(lxc|qemu)\.h)|(python/libvirt-override\.c)|(python/typewrappers\.h)$$
+ ^src/internal\.h$$
exclude_file_name_regexp--sc_prohibit_include_virterror_h = \
-
^(src/internal\.h)|(python/libvirt-|(lxc|qemu)-override\.c)|(python/typewrappers\.h)$$
+ ^src/internal\.h$$
+
+exclude_file_name_regexp--sc_prohibit_include_libvirt_h_1 = \
+ ^(examples/|tools/|python/|include/libvirt/)
+
+exclude_file_name_regexp--sc_prohibit_include_virterror_h_1 = \
+ ^(examples/|tools/|python/|include/libvirt/)
Maybe it's even worth merging 6 and 7 into a single patch, or splitting
the cleanup into a first patch, and the syntax check into a second.
+++ b/tests/shunloadhelper.c
@@ -28,8 +28,6 @@
#include <config.h>
#include "internal.h"
-#include <libvirt/libvirt.h>
-#include <libvirt/virterror.h>
#include <stdlib.h>
static void shunloadError(void *userData ATTRIBUTE_UNUSED,
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:41 a.m.
New subject: [libvirt] [PATCH 08/10] syntax-check: Don't include libvirt-{qemu, lxc}.h internally
Like libvirt.h, these two public headers also shouldn't be included
in internal source, except "internal.h". A later patch will force
tools to use <> form.
---
cfg.mk | 20 ++++++++++++++++++++
daemon/remote.c | 2 --
src/libvirt-qemu.c | 1 -
3 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 98c7e40..4b84bde 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -758,6 +758,20 @@ sc_prohibit_include_virterror_h:
halt='Do not include libvirt/virterror.h in internal source' \
$(_sc_search_regexp)
+# Don't include "libvirt/libvirt-qemu.h" in "" form.
+sc_prohibit_include_libvirt_qemu_h:
+ @prohibit='^# *include *"libvirt/libvirt-qemu\.h"' \
+ in_vc_files='\.[ch]$$' \
+ halt='Do not include libvirt/libvirt-qemu.h in internal source' \
+ $(_sc_search_regexp)
+
+# Don't include "libvirt/libvirt-lxc.h" in "" form.
+sc_prohibit_include_libvirt_lxc_h:
+ @prohibit='^# *include *"libvirt/libvirt-lxc\.h"' \
+ in_vc_files='\.[ch]$$' \
+ halt='Do not include libvirt/libvirt-lxc.h in internal source' \
+ $(_sc_search_regexp)
+
# Don't include "libvirt/libvirt.h" in <> form. Except external
tools, e.g.
# python binding, examples and tools subdirectories.
sc_prohibit_include_libvirt_h_1:
@@ -939,3 +953,9 @@ exclude_file_name_regexp--sc_prohibit_include_libvirt_h_1 = \
exclude_file_name_regexp--sc_prohibit_include_virterror_h_1 = \
^(examples/|tools/|python/|include/libvirt/)
+
+exclude_file_name_regexp--sc_prohibit_include_libvirt_qemu_h = \
+ ^(src/internal\.h$$|tools/)
+
+exclude_file_name_regexp--sc_prohibit_include_libvirt_lxc_h = \
+ ^(src/internal\.h$$|tools/)
diff --git a/daemon/remote.c b/daemon/remote.c
index 45c50f3..c559d6f 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -38,8 +38,6 @@
#include "virutil.h"
#include "stream.h"
#include "viruuid.h"
-#include "libvirt/libvirt-qemu.h"
-#include "libvirt/libvirt-lxc.h"
#include "vircommand.h"
#include "intprops.h"
#include "virnetserverservice.h"
diff --git a/src/libvirt-qemu.c b/src/libvirt-qemu.c
index 11da2f3..fb19584 100644
--- a/src/libvirt-qemu.c
+++ b/src/libvirt-qemu.c
@@ -26,7 +26,6 @@
#include "virerror.h"
#include "virlog.h"
#include "datatypes.h"
-#include "libvirt/libvirt-qemu.h"
#define VIR_FROM_THIS VIR_FROM_NONE
--
1.8.1.4
8:41 a.m.
New subject: [libvirt] [PATCH 09/10] syntax-check: Include libvirt-{qemu, lxc}.h in <> form only in external tools
Including libvirt-{qemu,lxc}.h in "" form is only allowed for
"internal.h"
now, and <> form is only allowed for external tools.
---
cfg.mk | 38 ++++++++++++++++++++++++++++++--------
tools/virsh.c | 4 ++--
2 files changed, 32 insertions(+), 10 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 4b84bde..4736d75 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -760,34 +760,50 @@ sc_prohibit_include_virterror_h:
# Don't include "libvirt/libvirt-qemu.h" in "" form.
sc_prohibit_include_libvirt_qemu_h:
- @prohibit='^# *include *"libvirt/libvirt-qemu\.h"' \
+ @prohibit='^# *include *"libvirt/libvirt-qemu\.h"' \
in_vc_files='\.[ch]$$' \
halt='Do not include libvirt/libvirt-qemu.h in internal source' \
$(_sc_search_regexp)
# Don't include "libvirt/libvirt-lxc.h" in "" form.
sc_prohibit_include_libvirt_lxc_h:
- @prohibit='^# *include *"libvirt/libvirt-lxc\.h"' \
+ @prohibit='^# *include *"libvirt/libvirt-lxc\.h"' \
in_vc_files='\.[ch]$$' \
halt='Do not include libvirt/libvirt-lxc.h in internal source' \
$(_sc_search_regexp)
-# Don't include "libvirt/libvirt.h" in <> form. Except external
tools, e.g.
-# python binding, examples and tools subdirectories.
+# Don't include "libvirt/libvirt.h" in <> form. Except for external
tools,
+# e.g. python binding, examples and tools subdirectories.
sc_prohibit_include_libvirt_h_1:
@prohibit='^# *include *<libvirt/libvirt\.h>' \
in_vc_files='\.[ch]$$' \
halt='Do not include libvirt/libvirt.h in internal source' \
$(_sc_search_regexp)
-# Don't include "libvirt/virterror.h" in <> form. Except external
tools, e.g.
-# python binding, examples and tools subdirectories.
+# Don't include "libvirt/virterror.h" in <> form. Except for external
tools,
+# e.g. python binding, examples and tools subdirectories.
sc_prohibit_include_virterror_h_1:
@prohibit='^# *include *<libvirt/virterror\.h>' \
in_vc_files='\.[ch]$$' \
halt='Do not include libvirt/virterror.h in internal source' \
$(_sc_search_regexp)
+# Don't include "libvirt/libvirt-qemu.h" in <> form. Except external
tools,
+# e.g. python binding, examples and tools subdirectories.
+sc_prohibit_include_libvirt_qemu_h_1:
+ @prohibit='^# *include *<libvirt/libvirt-qemu\.h>' \
+ in_vc_files='\.[ch]$$' \
+ halt='Do not include libvirt/libvirt-qemu.h in internal source' \
+ $(_sc_search_regexp)
+
+# Don't include "libvirt/libvirt-lxc.h" in <> form. Except external
tools,
+# e.g. python binding, examples and tools subdirectories.
+sc_prohibit_include_libvirt_lxc_h_1:
+ @prohibit='^# *include *<libvirt/libvirt-lxc\.h>' \
+ in_vc_files='\.[ch]$$' \
+ halt='Do not include libvirt/libvirt-lxc.h in internal source' \
+ $(_sc_search_regexp)
+
# We don't use this feature of maint.mk.
prev_version_file = /dev/null
@@ -955,7 +971,13 @@ exclude_file_name_regexp--sc_prohibit_include_virterror_h_1 = \
^(examples/|tools/|python/|include/libvirt/)
exclude_file_name_regexp--sc_prohibit_include_libvirt_qemu_h = \
- ^(src/internal\.h$$|tools/)
+ ^src/internal\.h$$
exclude_file_name_regexp--sc_prohibit_include_libvirt_lxc_h = \
- ^(src/internal\.h$$|tools/)
+ ^src/internal\.h$$
+
+exclude_file_name_regexp--sc_prohibit_include_libvirt_qemu_h_1 = \
+ ^(examples/|tools/|python/|include/libvirt/)
+
+exclude_file_name_regexp--sc_prohibit_include_libvirt_lxc_h_1 = \
+ ^(examples/|tools/|python/|include/libvirt/)
diff --git a/tools/virsh.c b/tools/virsh.c
index b7a5cc1..4cd93ca 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -60,8 +60,8 @@
#include "virutil.h"
#include "viralloc.h"
#include "virxml.h"
-#include "libvirt/libvirt-qemu.h"
-#include "libvirt/libvirt-lxc.h"
+#include <libvirt/libvirt-qemu.h>
+#include <libvirt/libvirt-lxc.h>
#include "virfile.h"
#include "configmake.h"
#include "virthread.h"
--
1.8.1.4
8:41 a.m.
New subject: [libvirt] [PATCH 10/10] docs: Update hacking
To tell libvirt-{qemu,lxc}.h shouldn't be included either.
---
docs/hacking.html.in | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 7ef826c..67af5c2 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -929,10 +929,10 @@
</pre>
<p>
- Of particular note: <b>Do not</b> include libvirt/libvirt.h or
- libvirt/virterror.h. It is included by "internal.h" already and there
- are some special reasons why you cannot include these files
- explicitly.
+ Of particular note: <b>Do not</b> include libvirt/libvirt.h,
+ libvirt/virterror.h, libvirt/libvirt-qemu.h, and libvirt/libvirt-lxc.h.
+ It is included by "internal.h" already and there are some special
reasons
+ why you cannot include these files explicitly.
</p>
--
1.8.1.4
4240
days inactive
4241
days old
24 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Osier Yang