[libvirt] [PATCH] Disallow inclusion of files from src/conf into src/utils

Peter Krempa posted 1 patch 7 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/5403b08570ca9a9f270def7304e3346acf453870.1487599097.git.pkrempa@redhat.com
cfg.mk                       | 3 +++
src/Makefile.am              | 3 +--
src/util/virclosecallbacks.h | 2 +-
src/util/virhostdev.h        | 2 +-
4 files changed, 6 insertions(+), 4 deletions(-)
[libvirt] [PATCH] Disallow inclusion of files from src/conf into src/utils
Posted by Peter Krempa 7 years, 1 month ago
The utils code should stay separated from other code (except for very
well justified cases). Unfortunately commit 272769beccd7479c75e700a6cb
made it trivial to break the separation (and not get slapped by the
syntax-check rule) by adding -I src/conf to the CFLAGS for utils.

Remove this shortcut and except the two offenders from the syntax check
so that the codebase can be kept separated.
---
 cfg.mk                       | 3 +++
 src/Makefile.am              | 3 +--
 src/util/virclosecallbacks.h | 2 +-
 src/util/virhostdev.h        | 2 +-
 4 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index 69e3f3a1a..aaba61f1d 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1242,3 +1242,6 @@ exclude_file_name_regexp--sc_prohibit_always-defined_macros = \

 exclude_file_name_regexp--sc_prohibit_readdir = \
   ^tests/.*mock\.c$$
+
+exclude_file_name_regexp--sc_prohibit_cross_inclusion = \
+  ^(src/util/virclosecallbacks\.h|src/util/virhostdev\.h)$$
diff --git a/src/Makefile.am b/src/Makefile.am
index 46ca272eb..4f3f8f0bd 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1137,8 +1137,7 @@ libvirt_util_la_SOURCES =					\
 libvirt_util_la_CFLAGS = $(CAPNG_CFLAGS) $(YAJL_CFLAGS) $(LIBNL_CFLAGS) \
 		$(AM_CFLAGS) $(AUDIT_CFLAGS) $(DEVMAPPER_CFLAGS) \
 		$(DBUS_CFLAGS) $(LDEXP_LIBM) $(NUMACTL_CFLAGS)	\
-		$(POLKIT_CFLAGS) $(GNUTLS_CFLAGS) $(ACL_CFLAGS) \
-		-I$(srcdir)/conf
+		$(POLKIT_CFLAGS) $(GNUTLS_CFLAGS) $(ACL_CFLAGS)
 libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) \
 		$(THREAD_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \
 		$(LIB_CLOCK_GETTIME) $(DBUS_LIBS) $(WIN32_EXTRA_LIBS) $(LIBXML_LIBS) \
diff --git a/src/util/virclosecallbacks.h b/src/util/virclosecallbacks.h
index 4df0a0060..d48997181 100644
--- a/src/util/virclosecallbacks.h
+++ b/src/util/virclosecallbacks.h
@@ -25,7 +25,7 @@
 #ifndef __VIR_CLOSE_CALLBACKS__
 # define __VIR_CLOSE_CALLBACKS__

-# include "virdomainobjlist.h"
+# include "conf/virdomainobjlist.h"

 typedef struct _virCloseCallbacks virCloseCallbacks;
 typedef virCloseCallbacks *virCloseCallbacksPtr;
diff --git a/src/util/virhostdev.h b/src/util/virhostdev.h
index 4c1fea3ef..1202136c2 100644
--- a/src/util/virhostdev.h
+++ b/src/util/virhostdev.h
@@ -31,7 +31,7 @@
 # include "virusb.h"
 # include "virscsi.h"
 # include "virscsivhost.h"
-# include "domain_conf.h"
+# include "conf/domain_conf.h"

 typedef enum {
     VIR_HOSTDEV_STRICT_ACS_CHECK     = (1 << 0), /* strict acs check */
-- 
2.11.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Disallow inclusion of files from src/conf into src/utils
Posted by Jiri Denemark 7 years, 1 month ago
On Mon, Feb 20, 2017 at 14:58:49 +0100, Peter Krempa wrote:
> The utils code should stay separated from other code (except for very
> well justified cases). Unfortunately commit 272769beccd7479c75e700a6cb
> made it trivial to break the separation (and not get slapped by the
> syntax-check rule) by adding -I src/conf to the CFLAGS for utils.
> 
> Remove this shortcut and except the two offenders from the syntax check
> so that the codebase can be kept separated.
> ---
>  cfg.mk                       | 3 +++
>  src/Makefile.am              | 3 +--
>  src/util/virclosecallbacks.h | 2 +-
>  src/util/virhostdev.h        | 2 +-
>  4 files changed, 6 insertions(+), 4 deletions(-)

ACK

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Disallow inclusion of files from src/conf into src/utils
Posted by Martin Kletzander 7 years, 1 month ago
On Mon, Feb 20, 2017 at 02:58:49PM +0100, Peter Krempa wrote:
>The utils code should stay separated from other code (except for very
>well justified cases). Unfortunately commit 272769beccd7479c75e700a6cb

I honestly don't see the justification for any of these, but ACK for
this, hopefully someone fixes the rest.

>made it trivial to break the separation (and not get slapped by the
>syntax-check rule) by adding -I src/conf to the CFLAGS for utils.
>
>Remove this shortcut and except the two offenders from the syntax check
>so that the codebase can be kept separated.
>---
> cfg.mk                       | 3 +++
> src/Makefile.am              | 3 +--
> src/util/virclosecallbacks.h | 2 +-
> src/util/virhostdev.h        | 2 +-
> 4 files changed, 6 insertions(+), 4 deletions(-)
>
>diff --git a/cfg.mk b/cfg.mk
>index 69e3f3a1a..aaba61f1d 100644
>--- a/cfg.mk
>+++ b/cfg.mk
>@@ -1242,3 +1242,6 @@ exclude_file_name_regexp--sc_prohibit_always-defined_macros = \
>
> exclude_file_name_regexp--sc_prohibit_readdir = \
>   ^tests/.*mock\.c$$
>+
>+exclude_file_name_regexp--sc_prohibit_cross_inclusion = \
>+  ^(src/util/virclosecallbacks\.h|src/util/virhostdev\.h)$$
>diff --git a/src/Makefile.am b/src/Makefile.am
>index 46ca272eb..4f3f8f0bd 100644
>--- a/src/Makefile.am
>+++ b/src/Makefile.am
>@@ -1137,8 +1137,7 @@ libvirt_util_la_SOURCES =					\
> libvirt_util_la_CFLAGS = $(CAPNG_CFLAGS) $(YAJL_CFLAGS) $(LIBNL_CFLAGS) \
> 		$(AM_CFLAGS) $(AUDIT_CFLAGS) $(DEVMAPPER_CFLAGS) \
> 		$(DBUS_CFLAGS) $(LDEXP_LIBM) $(NUMACTL_CFLAGS)	\
>-		$(POLKIT_CFLAGS) $(GNUTLS_CFLAGS) $(ACL_CFLAGS) \
>-		-I$(srcdir)/conf
>+		$(POLKIT_CFLAGS) $(GNUTLS_CFLAGS) $(ACL_CFLAGS)
> libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) \
> 		$(THREAD_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \
> 		$(LIB_CLOCK_GETTIME) $(DBUS_LIBS) $(WIN32_EXTRA_LIBS) $(LIBXML_LIBS) \
>diff --git a/src/util/virclosecallbacks.h b/src/util/virclosecallbacks.h
>index 4df0a0060..d48997181 100644
>--- a/src/util/virclosecallbacks.h
>+++ b/src/util/virclosecallbacks.h
>@@ -25,7 +25,7 @@
> #ifndef __VIR_CLOSE_CALLBACKS__
> # define __VIR_CLOSE_CALLBACKS__
>
>-# include "virdomainobjlist.h"
>+# include "conf/virdomainobjlist.h"
>
> typedef struct _virCloseCallbacks virCloseCallbacks;
> typedef virCloseCallbacks *virCloseCallbacksPtr;
>diff --git a/src/util/virhostdev.h b/src/util/virhostdev.h
>index 4c1fea3ef..1202136c2 100644
>--- a/src/util/virhostdev.h
>+++ b/src/util/virhostdev.h
>@@ -31,7 +31,7 @@
> # include "virusb.h"
> # include "virscsi.h"
> # include "virscsivhost.h"
>-# include "domain_conf.h"
>+# include "conf/domain_conf.h"
>
> typedef enum {
>     VIR_HOSTDEV_STRICT_ACS_CHECK     = (1 << 0), /* strict acs check */
>--
>2.11.0
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Disallow inclusion of files from src/conf into src/utils
Posted by Peter Krempa 7 years, 1 month ago
On Mon, Feb 20, 2017 at 15:05:33 +0100, Martin Kletzander wrote:
> On Mon, Feb 20, 2017 at 02:58:49PM +0100, Peter Krempa wrote:
> > The utils code should stay separated from other code (except for very
> > well justified cases). Unfortunately commit 272769beccd7479c75e700a6cb
> 
> I honestly don't see the justification for any of these, but ACK for
> this, hopefully someone fixes the rest.

My only justification is that I'm too lazy and too busy to fix them now
:).

Thanks I'll push it shortly.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Disallow inclusion of files from src/conf into src/utils
Posted by Martin Kletzander 7 years, 1 month ago
On Mon, Feb 20, 2017 at 03:11:37PM +0100, Peter Krempa wrote:
>On Mon, Feb 20, 2017 at 15:05:33 +0100, Martin Kletzander wrote:
>> On Mon, Feb 20, 2017 at 02:58:49PM +0100, Peter Krempa wrote:
>> > The utils code should stay separated from other code (except for very
>> > well justified cases). Unfortunately commit 272769beccd7479c75e700a6cb
>>
>> I honestly don't see the justification for any of these, but ACK for
>> this, hopefully someone fixes the rest.
>
>My only justification is that I'm too lazy and too busy to fix them now
>:).
>
>Thanks I'll push it shortly.

Oh, I thought there would be any.  Then, if this mail finds you before
you push it, please remove that information from the commit because the
commit itself might be a justification in the future for someone.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list