[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 01/16] xen/mm: Don't use _{g, m}fn for defining INVALID_{G, M}FN



Hi,

On 20/06/17 11:32, Jan Beulich wrote:
On 20.06.17 at 12:06, <tim@xxxxxxx> wrote:
At 03:36 -0600 on 20 Jun (1497929778), Jan Beulich wrote:
On 20.06.17 at 11:14, <tim@xxxxxxx> wrote:
At 01:32 -0600 on 20 Jun (1497922345), Jan Beulich wrote:
On 19.06.17 at 18:57, <julien.grall@xxxxxxx> wrote:
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -56,7 +56,7 @@

 TYPE_SAFE(unsigned long, mfn);
 #define PRI_mfn          "05lx"
-#define INVALID_MFN      _mfn(~0UL)
+#define INVALID_MFN      (mfn_t){ ~0UL }

While I don't expect anyone to wish to use a suffix expression on
this constant, for maximum compatibility this should still be fully
parenthesized, I think. Of course this should be easy enough to
do while committing.

Are you able to assure us that clang supports this gcc extension
(compound literal for non-compound types)

AIUI this is a C99 feature, not a GCCism.

Most parts of it yes (it is a gcc extension in C89 mode only), but the
specific use here isn't afaict: Compound literals outside of functions
are static objects, and hence couldn't be used as initializers of other
objects.

Ah, I see.  So would it be better to use

  #define INVALID_MFN ((const mfn_t) { ~0UL })

?

While I think we should indeed consider adding the const, the above
still is a static object, and hence still not suitable as an initializer as
per C99 or C11. But as long as gcc and clang permit it, we're fine.

Actually this solutions breaks on GCC 4.9 provided by Linaro ([1] 4.9-2016-02 and 4.9-2017.01).

This small reproducer does not compile with -std=gnu99 (used by Xen) but compile with this option. Jan, have you tried 4.9 with this patch?

typedef struct
{
    unsigned long i;
} mfn_t;

mfn_t v = (const mfn_t){~0UL};

Cheers,

[1] https://releases.linaro.org/components/toolchain/binaries/


Jan


--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.