[PATCH v2] libs/gnttab: introduce XENGNTTAB_BUILD_BUG_ON

[PATCH v2] libs/gnttab: introduce XENGNTTAB_BUILD_BUG_ON

[Wei Liu]

The implementation is taken from libxc.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>

I could have put it in a header file accessible to all libraries under
libs but this construct is only relevant to xengnttab library at the
moment so it's put under gnttab/private.h. It can be easily moved to
a common place when other libraries under libs require it.

This patch is necessary to unblock Paulina on her gnttab copy work.
---
 tools/libs/gnttab/private.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/libs/gnttab/private.h b/tools/libs/gnttab/private.h
index d286c86..2bdc0f2 100644
--- a/tools/libs/gnttab/private.h
+++ b/tools/libs/gnttab/private.h
@@ -4,6 +4,13 @@
 #include <xentoollog.h>
 #include <xengnttab.h>
 
+/* Force a compilation error if condition is true */
+#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
+#define XENGNTTAB_BUILD_BUG_ON(p) ({ _Static_assert(!(p), "!(" #p ")"); })
+#else
+#define XENGNTTAB_BUILD_BUG_ON(p) ((void)sizeof(struct { int:-!!(p); }))
+#endif
+
 struct xengntdev_handle {
     xentoollog_logger *logger, *logger_tofree;
     int fd;
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2] libs/gnttab: introduce XENGNTTAB_BUILD_BUG_ON

[Ian Jackson]

Wei Liu writes ("[PATCH v2] libs/gnttab: introduce XENGNTTAB_BUILD_BUG_ON"):
> The implementation is taken from libxc.
...
> I could have put it in a header file accessible to all libraries under
> libs but this construct is only relevant to xengnttab library at the
> moment so it's put under gnttab/private.h. It can be easily moved to
> a common place when other libraries under libs require it.

Other bits of tools already have this:

mariner:xen.git> git-grep -i 'define.*build_bug' tools | cat
tools/firmware/hvmloader/util.h:#define BUILD_BUG_ON(p) ((void)sizeof(char[1 - 2 * !!(p)]))
tools/libxc/xc_private.h:#define XC_BUILD_BUG_ON(p) ({ _Static_assert(!(p), "!(" #p ")"); })
tools/libxc/xc_private.h:#define XC_BUILD_BUG_ON(p) ((void)sizeof(struct { int:-!!(p); }))
tools/libxl/libxl_internal.h:#define BUILD_BUG_ON(p) ({ _Static_assert(!(p), "!(" #p ")"); })
tools/libxl/libxl_internal.h:#define BUILD_BUG_ON(p) ((void)sizeof(char[1 - 2 * !!(p)]))
mariner:xen.git> 

Do we really need to introduce yet another copy of this ?  Is gnttab
not allowed to include xc_private.h ?  Is there not some one place
where we could put this ?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2] libs/gnttab: introduce XENGNTTAB_BUILD_BUG_ON

[Wei Liu]

On Mon, Sep 19, 2016 at 12:41:47PM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH v2] libs/gnttab: introduce XENGNTTAB_BUILD_BUG_ON"):
> > The implementation is taken from libxc.
> ...
> > I could have put it in a header file accessible to all libraries under
> > libs but this construct is only relevant to xengnttab library at the
> > moment so it's put under gnttab/private.h. It can be easily moved to
> > a common place when other libraries under libs require it.
> 
> Other bits of tools already have this:
> 
> mariner:xen.git> git-grep -i 'define.*build_bug' tools | cat
> tools/firmware/hvmloader/util.h:#define BUILD_BUG_ON(p) ((void)sizeof(char[1 - 2 * !!(p)]))
> tools/libxc/xc_private.h:#define XC_BUILD_BUG_ON(p) ({ _Static_assert(!(p), "!(" #p ")"); })
> tools/libxc/xc_private.h:#define XC_BUILD_BUG_ON(p) ((void)sizeof(struct { int:-!!(p); }))
> tools/libxl/libxl_internal.h:#define BUILD_BUG_ON(p) ({ _Static_assert(!(p), "!(" #p ")"); })
> tools/libxl/libxl_internal.h:#define BUILD_BUG_ON(p) ((void)sizeof(char[1 - 2 * !!(p)]))
> mariner:xen.git> 
> 
> Do we really need to introduce yet another copy of this ?  Is gnttab
> not allowed to include xc_private.h ?  Is there not some one place
> where we could put this ?
> 

AIUI BUILD_BUG_ON is a private thing to each library, hence the
duplication. But I'm fine with having a central header for that, too.

If we want to share that macro across all libraries, we can create a
tools/include/private.h.  But that would require a fair bit of work --
adjust xen build system, fix stubdom build, fix mini-os etc -- which I
think it would be best to leave to me.

In the meantime, we should unblock Paulina on this one so that her
project can move forward.

Wei.

> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2] libs/gnttab: introduce XENGNTTAB_BUILD_BUG_ON

[Ian Jackson]

Wei Liu writes ("Re: [PATCH v2] libs/gnttab: introduce XENGNTTAB_BUILD_BUG_ON"):
> AIUI BUILD_BUG_ON is a private thing to each library, hence the
> duplication. But I'm fine with having a central header for that, too.
> 
> If we want to share that macro across all libraries, we can create a
> tools/include/private.h.  But that would require a fair bit of work --
> adjust xen build system, fix stubdom build, fix mini-os etc -- which I
> think it would be best to leave to me.
> 
> In the meantime, we should unblock Paulina on this one so that her
> project can move forward.

Well, you asked for my opinion.  I don't feel strongly enough to block
this patch.  I don't have any other objection to it.

The problem with the "we should unblock" approach is that as each
successive clone of this code is introduced, the task of fixing it up
becomes harder, and the precedent for further clones becomes stronger.

(As far as the rules for committing are concerned, I think this patch
can be committed with your ack but without mine.)

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2] libs/gnttab: introduce XENGNTTAB_BUILD_BUG_ON

[Wei Liu]

On Mon, Sep 19, 2016 at 02:49:05PM +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH v2] libs/gnttab: introduce XENGNTTAB_BUILD_BUG_ON"):
> > AIUI BUILD_BUG_ON is a private thing to each library, hence the
> > duplication. But I'm fine with having a central header for that, too.
> > 
> > If we want to share that macro across all libraries, we can create a
> > tools/include/private.h.  But that would require a fair bit of work --
> > adjust xen build system, fix stubdom build, fix mini-os etc -- which I
> > think it would be best to leave to me.
> > 
> > In the meantime, we should unblock Paulina on this one so that her
> > project can move forward.
> 
> Well, you asked for my opinion.  I don't feel strongly enough to block
> this patch.  I don't have any other objection to it.
> 
> The problem with the "we should unblock" approach is that as each
> successive clone of this code is introduced, the task of fixing it up
> becomes harder, and the precedent for further clones becomes stronger.
> 

I did try to fix all sort of things (this particular issue included) in
tools, stubdom and mini-os build system, but I didn't get to the point
to completely fix everything, unfortunately.

Previously attempt to fix leaking of BUILD_BUG_ON from mini-os resulted
in other breakage, then I ran out of my free cycles.

> (As far as the rules for committing are concerned, I think this patch
> can be committed with your ack but without mine.)
> 

I can't ack it, because I wrote this patch. :-)

Wei.

> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel