1:28 a.m.
*** BLURB HERE ***
Osier Yang (6):
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
cfg.mk | 34 ++++++++++++++++++++++++++++++++++
daemon/libvirtd.c | 1 -
src/conf/node_device_conf.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/shunloadtest.c | 4 ++--
tests/vmx2xmltest.c | 4 ++--
tests/xml2vmxtest.c | 4 ++--
31 files changed, 60 insertions(+), 45 deletions(-)
--
1.8.1.4
1:28 a.m.
New subject: [libvirt] [PATCH 1/6] cleanup: Remove the duplicate header
Detected by a simple Shell script.
---
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 728add5..4e76799 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 0d41e39..d15fb59 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:53 a.m.
New subject: [libvirt] [PATCH 1/6] cleanup: Remove the duplicate header
On 04/15/2013 12:28 AM, Osier Yang wrote:
Detected by a simple Shell script.
Mentioning the contents of that script in the commit message would make
it nicer to rerun the same check later, if we desired. In fact,
codifying the script into a syntax-check rule would prevent running into
this situation again (I haven't yet checked if you do that later in the
series, but if you do, then mentioning in _this_ commit that a later
commit will add a syntax-check would be helpful).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
1:28 a.m.
New subject: [libvirt] [PATCH 2/6] 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..0c6f3cb 100644
--- a/tests/esxutilstest.c
+++ b/tests/esxutilstest.c
@@ -1,5 +1,7 @@
#include <config.h>
+#include "virutil.h"
+
#ifdef WITH_ESX
# include <stdio.h>
@@ -9,7 +11,6 @@
# include "internal.h"
# include "viralloc.h"
# include "testutils.h"
-# include "virutil.h"
# include "vmx/vmx.h"
# include "esx/esx_util.h"
# include "esx/esx_vi_types.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
6:23 a.m.
New subject: [libvirt] [PATCH 2/6] cleanup: Only include testutils.h once
diff --git a/tests/esxutilstest.c b/tests/esxutilstest.c
index 55b3623..0c6f3cb 100644
--- a/tests/esxutilstest.c
+++ b/tests/esxutilstest.c
@@ -1,5 +1,7 @@
#include <config.h>
+#include "virutil.h"
+
#ifdef WITH_ESX
# include <stdio.h>
@@ -9,7 +11,6 @@
# include "internal.h"
# include "viralloc.h"
# include "testutils.h"
-# include "virutil.h"
# include "vmx/vmx.h"
# include "esx/esx_util.h"
# include "esx/esx_vi_types.h"
@@ -279,7 +280,6 @@ mymain(void)
VIRT_TEST_MAIN(mymain)
#else
-# include "testutils.h"
int main(void)
{
You moved "virutil.h" instead of "testutils.h" here.
Jan
1:28 a.m.
New subject: [libvirt] [PATCH 3/6] cleanup: Don't include libvirt/libvirt.h
Which is already included by "internal.h".
---
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
1:28 a.m.
New subject: [libvirt] [PATCH 4/6] cleanup: Don't include libvirt/virterror.h
---
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
1:28 a.m.
New subject: [libvirt] [PATCH 5/6] syntax-check: Don't include duplicate header
---
cfg.mk | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/cfg.mk b/cfg.mk
index 394521e..9cf4cff 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -722,6 +722,20 @@ sc_prohibit_exit_in_tests:
halt='use return, not exit(), in tests' \
$(_sc_search_regexp)
+# Don't include duplidate header in the source (either *.c or *.h)
+sc_prohibit_duplicate_header:
+ @if $(VC_LIST_EXCEPT) | grep -l '\.[ch]$$' > /dev/null; then \
+ for i in $$($(VC_LIST_EXCEPT) | grep '\.[ch]$$'); do \
+ for j in $$(grep '^# *include.*\.h' $$i \
+ | awk -F' ' '{print $$NF}'); do \
+ test $$(grep "^# *include $$j" $$i | wc -l) -gt 1 && \
+ { echo '$(ME): Duplidate header '$$j' in '$$i'' 1>&2; \
+ exit 1; } || :; \
+ done; \
+ done; \
+ else :; \
+ fi
+
# We don't use this feature of maint.mk.
prev_version_file = /dev/null
--
1.8.1.4
10:11 a.m.
New subject: [libvirt] [PATCH 5/6] syntax-check: Don't include duplicate header
On 04/15/2013 12:28 AM, Osier Yang wrote:
---
cfg.mk | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/cfg.mk b/cfg.mk
index 394521e..9cf4cff 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -722,6 +722,20 @@ sc_prohibit_exit_in_tests:
halt='use return, not exit(), in tests' \
$(_sc_search_regexp)
+# Don't include duplidate header in the source (either *.c or *.h)
s/duplidate/duplicate/
+sc_prohibit_duplicate_header:
+ @if $(VC_LIST_EXCEPT) | grep -l '\.[ch]$$' > /dev/null; then \
'grep -l' is wrong. You want:
grep '\.[ch]$$'
instead (see sc_preprocessor_indenation for an example).
For that matter, the '@if ...; then check; else :; fi' is overkill; we
KNOW we have .c files so the grep will hit (that paradigm is used in
maint.mk, because maint.mk is shared among multiple projects where some
projects really do ship without C files). I'd simplify this to just the
'check' portion, that is, all you need is from here...
+ for i in $$($(VC_LIST_EXCEPT) | grep '\.[ch]$$'); do \
+ for j in $$(grep '^# *include.*\.h' $$i \
+ | awk -F' ' '{print $$NF}'); do \
+ test $$(grep "^# *include $$j" $$i | wc -l) -gt 1 && \
Not the most efficient way to write this. Your way does one grep/wc
pair per header line encountered per file. But since you are already
running each file through awk, why not have awk set up a hash of all
includes it sees, and then report an error if the hash hits more than
once, all on a single awk pass per file instead of 20-30 grep passes per
file. Would you like to take a shot at it, or shall I do it since I
mentioned it?
+ { echo '$(ME): Duplidate header '$$j' in
'$$i'' 1>&2; \
s/Duplidate/Duplicate/
+ exit 1; } || :; \
+ done; \
+ done; \
...to here.
+ else :; \
+ fi
+
# We don't use this feature of maint.mk.
prev_version_file = /dev/null
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
5:46 a.m.
New subject: [libvirt] [PATCH 5/6] syntax-check: Don't include duplicate header
On 15/04/13 23:11, Eric Blake wrote:
On 04/15/2013 12:28 AM, Osier Yang wrote:
> ---
> cfg.mk | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/cfg.mk b/cfg.mk
> index 394521e..9cf4cff 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -722,6 +722,20 @@ sc_prohibit_exit_in_tests:
> halt='use return, not exit(), in tests' \
> $(_sc_search_regexp)
>
> +# Don't include duplidate header in the source (either *.c or *.h)
s/duplidate/duplicate/
> +sc_prohibit_duplicate_header:
> + @if $(VC_LIST_EXCEPT) | grep -l '\.[ch]$$' > /dev/null; then \
'grep -l' is wrong. You want:
grep '\.[ch]$$'
instead (see sc_preprocessor_indenation for an example).
For that matter, the '@if ...; then check; else :; fi' is overkill; we
KNOW we have .c files so the grep will hit (that paradigm is used in
maint.mk, because maint.mk is shared among multiple projects where some
projects really do ship without C files). I'd simplify this to just the
'check' portion, that is, all you need is from here...
In case of duplicate work, I finished this, but need more work on
6/6 to send out the set.
> + for i in $$($(VC_LIST_EXCEPT) | grep '\.[ch]$$'); do \
> + for j in $$(grep '^# *include.*\.h' $$i \
> + | awk -F' ' '{print $$NF}'); do \
> + test $$(grep "^# *include $$j" $$i | wc -l) -gt 1 && \
Not the most efficient way to write this. Your way does one grep/wc
pair per header line encountered per file. But since you are already
running each file through awk, why not have awk set up a hash of all
includes it sees, and then report an error if the hash hits more than
once, all on a single awk pass per file instead of 20-30 grep passes per
file. Would you like to take a shot at it, or shall I do it since I
mentioned it?
> + { echo '$(ME): Duplidate header '$$j' in '$$i''
1>&2; \
s/Duplidate/Duplicate/
> + exit 1; } || :; \
> + done; \
> + done; \
...to here.
> + else :; \
> + fi
> +
> # We don't use this feature of maint.mk.
> prev_version_file = /dev/null
>
>
1:28 a.m.
New subject: [libvirt] [PATCH 6/6] syntax-check: Don't include libvirt.h and virterror.h
Except for "internal.h" and the public headers
---
cfg.mk | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/cfg.mk b/cfg.mk
index 9cf4cff..0f07abd 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -736,6 +736,20 @@ sc_prohibit_duplicate_header:
else :; \
fi
+# Don't include "libvirt/libvirt.h".
+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".
+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
@@ -888,3 +902,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:18 a.m.
New subject: [libvirt] [PATCH 6/6] syntax-check: Don't include libvirt.h and virterror.h
On 04/15/2013 12:28 AM, Osier Yang wrote:
Except for "internal.h" and the public headers
---
cfg.mk | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/cfg.mk b/cfg.mk
index 9cf4cff..0f07abd 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -736,6 +736,20 @@ sc_prohibit_duplicate_header:
else :; \
fi
+# Don't include "libvirt/libvirt.h".
+sc_prohibit_include_libvirt_h:
+ @prohibit='^# *include "libvirt/libvirt.h"' \
This prohibits "libvirt/libvirt.h", but allows <libvirt/libvirt.h>. The
distinction is important, the "" form searches the current directory
first, while the <> form does not. Since we are stating that this
header is public, I think you're trying to state that internal files
should never include this file directly (no "" or <> form, but get its
contents through "internal.h" instead), while external tools (python
bindings, tools subdirectory) which should be limited to just the public
API should be using the <> form.
+ in_vc_files='\.[ch]$$' \
+ halt='Do not include libvirt/libvirt.h' \
+ $(_sc_search_regexp)
+
+# Don't include "libvirt/virterror.h".
+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
@@ -888,3 +902,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)$$
I'm wondering if instead of excluding so many files, if we should
instead be converting these files to use the <> include form. I'm also
wondering if your syntax check needs to check for no "" form anywhere,
and no <> form in daemon or src subdirectories (while still allowing <>
form in tools and python subdirectories).
+
+exclude_file_name_regexp--sc_prohibit_include_virterror_h = \
+
^(src/internal\.h)|(python/libvirt-|(lxc|qemu)-override\.c)|(python/typewrappers\.h)$$
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:44 a.m.
New subject: [libvirt] [PATCH 6/6] syntax-check: Don't include libvirt.h and virterror.h
On 15/04/13 23:18, Eric Blake wrote:
On 04/15/2013 12:28 AM, Osier Yang wrote:
> Except for "internal.h" and the public headers
> ---
> cfg.mk | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/cfg.mk b/cfg.mk
> index 9cf4cff..0f07abd 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -736,6 +736,20 @@ sc_prohibit_duplicate_header:
> else :; \
> fi
>
> +# Don't include "libvirt/libvirt.h".
> +sc_prohibit_include_libvirt_h:
> + @prohibit='^# *include "libvirt/libvirt.h"' \
This prohibits "libvirt/libvirt.h", but allows <libvirt/libvirt.h>. The
distinction is important, the "" form searches the current directory
first, while the <> form does not. Since we are stating that this
header is public, I think you're trying to state that internal files
should never include this file directly (no "" or <> form, but get its
contents through "internal.h" instead), while external tools (python
bindings, tools subdirectory) which should be limited to just the public
API should be using the <> form.
Yeah, that was what I tended to do, posted v2, including the syntax-check
rules for libvirt-{qemu,lxc}.h too.
> + in_vc_files='\.[ch]$$' \
> + halt='Do not include libvirt/libvirt.h' \
> + $(_sc_search_regexp)
> +
> +# Don't include "libvirt/virterror.h".
> +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
>
> @@ -888,3 +902,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)$$
I'm wondering if instead of excluding so many files, if we should
instead be converting these files to use the <> include form. I'm also
wondering if your syntax check needs to check for no "" form anywhere,
and no <> form in daemon or src subdirectories (while still allowing <>
form in tools and python subdirectories).
> +
> +exclude_file_name_regexp--sc_prohibit_include_virterror_h = \
> +
^(src/internal\.h)|(python/libvirt-|(lxc|qemu)-override\.c)|(python/typewrappers\.h)$$
>
5:51 a.m.
On 04/15/2013 02:28 AM, Osier Yang wrote:
*** BLURB HERE ***
Osier Yang (6):
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
cfg.mk | 34 ++++++++++++++++++++++++++++++++++
daemon/libvirtd.c | 1 -
src/conf/node_device_conf.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/shunloadtest.c | 4 ++--
tests/vmx2xmltest.c | 4 ++--
tests/xml2vmxtest.c | 4 ++--
31 files changed, 60 insertions(+), 45 deletions(-)
Mechanically the series looks fine to me.
Although in 6/6 do the '@prohibit' and 'halt' lines need to have the
period escaped for libvirt.h and virterror.h? It seems the prohibit
entry might as I see another example "@prohibit='^# *include
*<ctype\.h>'"
John
6:01 a.m.
On 15/04/13 18:51, John Ferlan wrote:
On 04/15/2013 02:28 AM, Osier Yang wrote:
> *** BLURB HERE ***
>
> Osier Yang (6):
> 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
>
> cfg.mk | 34 ++++++++++++++++++++++++++++++++++
> daemon/libvirtd.c | 1 -
> src/conf/node_device_conf.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/shunloadtest.c | 4 ++--
> tests/vmx2xmltest.c | 4 ++--
> tests/xml2vmxtest.c | 4 ++--
> 31 files changed, 60 insertions(+), 45 deletions(-)
>
Mechanically the series looks fine to me.
Although in 6/6 do the '@prohibit' and 'halt' lines need to have the
period escaped for libvirt.h and virterror.h? It seems the prohibit
entry might as I see another example "@prohibit='^# *include
*<ctype\.h>'"
Yeah, it needs, I guess it works fine just because there is no
strings like (#include "libvirt/libvirt5h") in the source. Will change,
thanks.
Osier
4268
days inactive
4269
days old
14 comments
4 participants
participants (4)
-
Eric Blake -
John Ferlan -
Ján Tomko -
Osier Yang