[Qemu-devel] [PATCH v2 0/4] Allow QOM struct fields to be marked as private

[Qemu-devel] [PATCH v2 0/4] Allow QOM struct fields to be marked as private

[Peter Maydell]

This patch series provides infrastructure and documentation
for marking QOM struct fields as private to the class implementation.

Patch 1 is the implementation (which is a trivial five lines!) and
documentation of the code pattern that it's intended to be used with.

Patches 2, 3 and 4 are examples of its use: patch 2 deals with
the ARM GIC classes, as an example of a largish class with some
subclasses. Patches 3 and 4 are conversions of much simpler and
smaller devices, so might be easier to look at first.

A .h file which uses this pattern ends up with half a dozen
extra lines of boilerplate, which is slightly sad but not too
awful. It would be pretty easy to autogenerate (along with the
type macros themselves) if we decided to do that in future, though.

Example of the compiler message if you try to touch a field
which is private:
 arm_gic_kvm.c:559:5: error: ‘iomem’ is deprecated (declared at /root/qemu/include/hw/intc/arm_gic_common.h:105): this field is private [-Werror=deprecated-declarations]


Changes since the RFC back in July last year:
 * name the macro 'qom_private' rather than '__private'
 * redid patch 2 as the GIC code has been rearranged somewhat
 * patches 3 and 4 are new

Peter Maydell (4):
  Provide infrastructure for marking private QOM struct fields
  arm_gic: Use new qom_private macro to mark private fields
  a9scu: Use qom_private to mark private fields
  arm11scu: Use qom_private to mark private fields

 hw/intc/arm_gic.c                |  3 ++
 hw/intc/arm_gic_common.c         |  2 ++
 hw/intc/arm_gic_kvm.c            |  2 ++
 hw/intc/armv7m_nvic.c            |  2 ++
 hw/misc/a9scu.c                  |  2 ++
 hw/misc/arm11scu.c               |  2 ++
 include/hw/intc/arm_gic.h        | 12 ++++++--
 include/hw/intc/arm_gic_common.h | 62 +++++++++++++++++++++++-----------------
 include/hw/misc/a9scu.h          | 16 +++++++----
 include/hw/misc/arm11scu.h       | 14 ++++++---
 include/qemu/compiler.h          | 10 +++++++
 include/qom/object.h             | 47 ++++++++++++++++++++++++++++++
 12 files changed, 136 insertions(+), 38 deletions(-)

-- 
1.9.2

[Qemu-devel] [PATCH v2 1/4] Provide infrastructure for marking private QOM struct fields

[Peter Maydell]

Provide infrastructure for marking private QOM struct fields,
so that a compiler warning is generated when a user of the QOM
object attempts to access them directly.

This is implemented using GCC's 'deprecated' attribute; preprocessor
macros arrange that when compiling the class implementation,
no attribute is applied to the fields; when compiling a user
of the class the fields are marked deprecated.

This allows us to have a single simple C struct defining the
object, and for users of the QOM object to be able to embed
instances of it into other structs, but still to have a guard
against users accidentally touching parts of the structure
they should not be accessing.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/qemu/compiler.h | 10 ++++++++++
 include/qom/object.h    | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+)

diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index 155b358..d7cc153 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -52,4 +52,14 @@
 #define GCC_FMT_ATTR(n, m)
 #endif
 
+/* An attribute usable to mark structure fields as private to the
+ * implementation; since this is only a diagnostic to catch programming
+ * errors, it's OK if it expands to nothing on non-gcc compilers.
+ */
+#if defined __GNUC__
+# define QEMU_PRIVATE_ATTR __attribute__((deprecated("this field is private")))
+#else
+# define QEMU_PRIVATE_ATTR
+#endif
+
 #endif /* COMPILER_H */
diff --git a/include/qom/object.h b/include/qom/object.h
index a641dcd..ea80008 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -284,6 +284,53 @@ typedef struct InterfaceInfo InterfaceInfo;
  *
  * The first example of such a QOM method was #CPUClass.reset,
  * another example is #DeviceClass.realize.
+ *
+ * = Marking fields as private to the class implementation =
+ *
+ * The expected code structure for QOM objects is that they should
+ * have a header file in include/ which defines the class and object
+ * structures and the typecasting macros. This header can then be
+ * included by both the source file which implements the QOM object
+ * and also by other source files which merely wish to use the object.
+ * Users of your object need the class and object structures so that
+ * they can embed instances of the object in their own structures;
+ * however they do not need to be able to access individual fields in
+ * these structures. To enforce this you should use the QEMU_PRIVATE_ATTR
+ * macro in a pattern like this:
+ *
+ * <example>
+ *   <title>Marking fields as private</title>
+ *   <programlisting>
+ * #ifdef IMPLEMENTING_MY_DEVICE
+ * # define qom_private
+ * #else
+ * # define qom_private QEMU_PRIVATE_ATTR
+ * #endif
+ *
+ * typedef struct MyDevice
+ * {
+ *     qom_private DeviceState parent;
+ *
+ *     qom_private int reg0, reg1, reg2;
+ * } MyDevice;
+ *
+ * typedef struct MyDeviceClass
+ * {
+ *     qom_private DeviceClass parent;
+ *
+ *     void (*frobnicate) (MyDevice *obj);
+ * } MyDeviceClass;
+ *
+ * #undef qom_private
+ *   </programlisting>
+ * </example>
+ *
+ * The source files which provide the implementation of your
+ * class (or of subclasses to it) should then have
+ * "#define IMPLEMENTING_MY_DEVICE" before they include any
+ * headers. Since users of the class will not define this
+ * macro, they will get a compilation warning if they access
+ * any of the private fields by mistake.
  */
 
 
-- 
1.9.2

[Qemu-devel] [PATCH v2 2/4] arm_gic: Use new qom_private macro to mark private fields

[Peter Maydell]

Use the new qom_private macro infrastructure to mark private fields for
the arm_gic classes.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/intc/arm_gic.c                |  3 ++
 hw/intc/arm_gic_common.c         |  2 ++
 hw/intc/arm_gic_kvm.c            |  2 ++
 hw/intc/armv7m_nvic.c            |  2 ++
 include/hw/intc/arm_gic.h        | 12 ++++++--
 include/hw/intc/arm_gic_common.h | 62 +++++++++++++++++++++++-----------------
 6 files changed, 54 insertions(+), 29 deletions(-)

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 1532ef9..afe2179 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -18,6 +18,9 @@
  *  armv7m_nvic device.
  */
 
+#define IMPLEMENTING_ARM_GIC
+#define IMPLEMENTING_ARM_GIC_COMMON
+
 #include "hw/sysbus.h"
 #include "gic_internal.h"
 #include "qom/cpu.h"
diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
index 6d884ec..8a2c434 100644
--- a/hw/intc/arm_gic_common.c
+++ b/hw/intc/arm_gic_common.c
@@ -18,6 +18,8 @@
  * with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#define IMPLEMENTING_ARM_GIC_COMMON
+
 #include "gic_internal.h"
 
 static void gic_pre_save(void *opaque)
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index 5038885..792073e 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -19,6 +19,8 @@
  * with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#define IMPLEMENTING_ARM_GIC_COMMON
+
 #include "hw/sysbus.h"
 #include "sysemu/kvm.h"
 #include "kvm_arm.h"
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 9aa8ab2..d68b286 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -10,6 +10,8 @@
  * NVIC.  Much of that is also implemented here.
  */
 
+#define IMPLEMENTING_ARM_GIC_COMMON
+
 #include "hw/sysbus.h"
 #include "qemu/timer.h"
 #include "hw/arm/arm.h"
diff --git a/include/hw/intc/arm_gic.h b/include/hw/intc/arm_gic.h
index 0971e37..8d69e02 100644
--- a/include/hw/intc/arm_gic.h
+++ b/include/hw/intc/arm_gic.h
@@ -31,12 +31,20 @@
 #define ARM_GIC_GET_CLASS(obj) \
      OBJECT_GET_CLASS(ARMGICClass, (obj), TYPE_ARM_GIC)
 
+#ifdef IMPLEMENTING_ARM_GIC
+#define qom_private
+#else
+#define qom_private QEMU_PRIVATE_ATTR
+#endif
+
 typedef struct ARMGICClass {
     /*< private >*/
-    ARMGICCommonClass parent_class;
+    qom_private ARMGICCommonClass parent_class;
     /*< public >*/
 
-    DeviceRealize parent_realize;
+    qom_private DeviceRealize parent_realize;
 } ARMGICClass;
 
+#undef qom_private
+
 #endif
diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h
index f6887ed..39211bd 100644
--- a/include/hw/intc/arm_gic_common.h
+++ b/include/hw/intc/arm_gic_common.h
@@ -44,39 +44,45 @@ typedef struct gic_irq_state {
     bool edge_trigger; /* true: edge-triggered, false: level-triggered  */
 } gic_irq_state;
 
+#ifdef IMPLEMENTING_ARM_GIC_COMMON
+#define qom_private
+#else
+#define qom_private QEMU_PRIVATE_ATTR
+#endif
+
 typedef struct GICState {
     /*< private >*/
-    SysBusDevice parent_obj;
+    qom_private SysBusDevice parent_obj;
     /*< public >*/
 
-    qemu_irq parent_irq[GIC_NCPU];
-    bool enabled;
-    bool cpu_enabled[GIC_NCPU];
+    qom_private qemu_irq parent_irq[GIC_NCPU];
+    qom_private bool enabled;
+    qom_private bool cpu_enabled[GIC_NCPU];
 
-    gic_irq_state irq_state[GIC_MAXIRQ];
-    uint8_t irq_target[GIC_MAXIRQ];
-    uint8_t priority1[GIC_INTERNAL][GIC_NCPU];
-    uint8_t priority2[GIC_MAXIRQ - GIC_INTERNAL];
-    uint16_t last_active[GIC_MAXIRQ][GIC_NCPU];
+    qom_private gic_irq_state irq_state[GIC_MAXIRQ];
+    qom_private uint8_t irq_target[GIC_MAXIRQ];
+    qom_private uint8_t priority1[GIC_INTERNAL][GIC_NCPU];
+    qom_private uint8_t priority2[GIC_MAXIRQ - GIC_INTERNAL];
+    qom_private uint16_t last_active[GIC_MAXIRQ][GIC_NCPU];
     /* For each SGI on the target CPU, we store 8 bits
      * indicating which source CPUs have made this SGI
      * pending on the target CPU. These correspond to
      * the bytes in the GIC_SPENDSGIR* registers as
      * read by the target CPU.
      */
-    uint8_t sgi_pending[GIC_NR_SGIS][GIC_NCPU];
+    qom_private uint8_t sgi_pending[GIC_NR_SGIS][GIC_NCPU];
 
-    uint16_t priority_mask[GIC_NCPU];
-    uint16_t running_irq[GIC_NCPU];
-    uint16_t running_priority[GIC_NCPU];
-    uint16_t current_pending[GIC_NCPU];
+    qom_private uint16_t priority_mask[GIC_NCPU];
+    qom_private uint16_t running_irq[GIC_NCPU];
+    qom_private uint16_t running_priority[GIC_NCPU];
+    qom_private uint16_t current_pending[GIC_NCPU];
 
     /* We present the GICv2 without security extensions to a guest and
      * therefore the guest can configure the GICC_CTLR to configure group 1
      * binary point in the abpr.
      */
-    uint8_t  bpr[GIC_NCPU];
-    uint8_t  abpr[GIC_NCPU];
+    qom_private uint8_t  bpr[GIC_NCPU];
+    qom_private uint8_t  abpr[GIC_NCPU];
 
     /* The APR is implementation defined, so we choose a layout identical to
      * the KVM ABI layout for QEMU's implementation of the gic:
@@ -92,19 +98,19 @@ typedef struct GICState {
      * do power management involving powering down and restarting
      * the GIC.
      */
-    uint32_t apr[GIC_NR_APRS][GIC_NCPU];
+    qom_private uint32_t apr[GIC_NR_APRS][GIC_NCPU];
 
-    uint32_t num_cpu;
+    qom_private uint32_t num_cpu;
 
-    MemoryRegion iomem; /* Distributor */
+    qom_private MemoryRegion iomem; /* Distributor */
     /* This is just so we can have an opaque pointer which identifies
      * both this GIC and which CPU interface we should be accessing.
      */
-    struct GICState *backref[GIC_NCPU];
-    MemoryRegion cpuiomem[GIC_NCPU + 1]; /* CPU interfaces */
-    uint32_t num_irq;
-    uint32_t revision;
-    int dev_fd; /* kvm device fd if backed by kvm vgic support */
+    qom_private struct GICState *backref[GIC_NCPU];
+    qom_private MemoryRegion cpuiomem[GIC_NCPU + 1]; /* CPU interfaces */
+    qom_private uint32_t num_irq;
+    qom_private uint32_t revision;
+    qom_private int dev_fd; /* kvm device fd if backed by kvm vgic support */
 } GICState;
 
 #define TYPE_ARM_GIC_COMMON "arm_gic_common"
@@ -117,11 +123,13 @@ typedef struct GICState {
 
 typedef struct ARMGICCommonClass {
     /*< private >*/
-    SysBusDeviceClass parent_class;
+    qom_private SysBusDeviceClass parent_class;
     /*< public >*/
 
-    void (*pre_save)(GICState *s);
-    void (*post_load)(GICState *s);
+    qom_private void (*pre_save)(GICState *s);
+    qom_private void (*post_load)(GICState *s);
 } ARMGICCommonClass;
 
+#undef qom_private
+
 #endif
-- 
1.9.2

[Qemu-devel] [PATCH v2 3/4] a9scu: Use qom_private to mark private fields

[Peter Maydell]

Use the new qom_private infrastructure to mark private fields
in the QOM device struct.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/misc/a9scu.c         |  2 ++
 include/hw/misc/a9scu.h | 16 +++++++++++-----
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/hw/misc/a9scu.c b/hw/misc/a9scu.c
index 4434945..81587b6 100644
--- a/hw/misc/a9scu.c
+++ b/hw/misc/a9scu.c
@@ -8,6 +8,8 @@
  * This code is licensed under the GPL.
  */
 
+#define IMPLEMENTING_A9_SCU
+
 #include "hw/misc/a9scu.h"
 
 static uint64_t a9_scu_read(void *opaque, hwaddr offset,
diff --git a/include/hw/misc/a9scu.h b/include/hw/misc/a9scu.h
index efb0c30..86dd21f 100644
--- a/include/hw/misc/a9scu.h
+++ b/include/hw/misc/a9scu.h
@@ -14,15 +14,21 @@
 
 /* A9MP private memory region.  */
 
+#ifdef IMPLEMENTING_A9_SCU
+#define qom_private
+#else
+#define qom_private QEMU_PRIVATE_ATTR
+#endif
+
 typedef struct A9SCUState {
     /*< private >*/
-    SysBusDevice parent_obj;
+    qom_private SysBusDevice parent_obj;
     /*< public >*/
 
-    MemoryRegion iomem;
-    uint32_t control;
-    uint32_t status;
-    uint32_t num_cpu;
+    qom_private MemoryRegion iomem;
+    qom_private uint32_t control;
+    qom_private uint32_t status;
+    qom_private uint32_t num_cpu;
 } A9SCUState;
 
 #define TYPE_A9_SCU "a9-scu"
-- 
1.9.2

[Qemu-devel] [PATCH v2 4/4] arm11scu: Use qom_private to mark private fields

[Peter Maydell]

Mark the arm11scu private fields as qom_private.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/misc/arm11scu.c         |  2 ++
 include/hw/misc/arm11scu.h | 14 ++++++++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/misc/arm11scu.c b/hw/misc/arm11scu.c
index a791675..d8d5b2b 100644
--- a/hw/misc/arm11scu.c
+++ b/hw/misc/arm11scu.c
@@ -8,6 +8,8 @@
  * This code is licensed under the GPL.
  */
 
+#define IMPLEMENTING_ARM11_SCU
+
 #include "hw/misc/arm11scu.h"
 
 static uint64_t mpcore_scu_read(void *opaque, hwaddr offset,
diff --git a/include/hw/misc/arm11scu.h b/include/hw/misc/arm11scu.h
index 5ad0f3d..4de1eb1 100644
--- a/include/hw/misc/arm11scu.h
+++ b/include/hw/misc/arm11scu.h
@@ -16,14 +16,20 @@
 #define TYPE_ARM11_SCU "arm11-scu"
 #define ARM11_SCU(obj) OBJECT_CHECK(ARM11SCUState, (obj), TYPE_ARM11_SCU)
 
+#ifdef IMPLEMENTING_ARM11_SCU
+#define qom_private
+#else
+#define qom_private QEMU_PRIVATE_ATTR
+#endif
+
 typedef struct ARM11SCUState {
     /*< private >*/
-    SysBusDevice parent_obj;
+    qom_private SysBusDevice parent_obj;
     /*< public >*/
 
-    uint32_t control;
-    uint32_t num_cpu;
-    MemoryRegion iomem;
+    qom_private uint32_t control;
+    qom_private uint32_t num_cpu;
+    qom_private MemoryRegion iomem;
 } ARM11SCUState;
 
 #endif
-- 
1.9.2

Re: [Qemu-devel] [PATCH v2 0/4] Allow QOM struct fields to be marked as private

[Peter Maydell]

On 9 May 2014 16:56, Peter Maydell <peter.maydell@linaro.org> wrote:
> This patch series provides infrastructure and documentation
> for marking QOM struct fields as private to the class implementation.
>
> Patch 1 is the implementation (which is a trivial five lines!) and
> documentation of the code pattern that it's intended to be used with.
>
> Patches 2, 3 and 4 are examples of its use: patch 2 deals with
> the ARM GIC classes, as an example of a largish class with some
> subclasses. Patches 3 and 4 are conversions of much simpler and
> smaller devices, so might be easier to look at first.
>
> A .h file which uses this pattern ends up with half a dozen
> extra lines of boilerplate, which is slightly sad but not too
> awful. It would be pretty easy to autogenerate (along with the
> type macros themselves) if we decided to do that in future, though.
>
> Example of the compiler message if you try to touch a field
> which is private:
>  arm_gic_kvm.c:559:5: error: ‘iomem’ is deprecated (declared at /root/qemu/include/hw/intc/arm_gic_common.h:105): this field is private [-Werror=deprecated-declarations]

Ping?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 0/4] Allow QOM struct fields to be marked as private

[Andreas Färber]

Am 23.05.2014 13:13, schrieb Peter Maydell:
> On 9 May 2014 16:56, Peter Maydell <peter.maydell@linaro.org> wrote:
>> This patch series provides infrastructure and documentation
>> for marking QOM struct fields as private to the class implementation.
>>
>> Patch 1 is the implementation (which is a trivial five lines!) and
>> documentation of the code pattern that it's intended to be used with.
>>
>> Patches 2, 3 and 4 are examples of its use: patch 2 deals with
>> the ARM GIC classes, as an example of a largish class with some
>> subclasses. Patches 3 and 4 are conversions of much simpler and
>> smaller devices, so might be easier to look at first.
>>
>> A .h file which uses this pattern ends up with half a dozen
>> extra lines of boilerplate, which is slightly sad but not too
>> awful. It would be pretty easy to autogenerate (along with the
>> type macros themselves) if we decided to do that in future, though.
>>
>> Example of the compiler message if you try to touch a field
>> which is private:
>>  arm_gic_kvm.c:559:5: error: ‘iomem’ is deprecated (declared at /root/qemu/include/hw/intc/arm_gic_common.h:105): this field is private [-Werror=deprecated-declarations]
> 
> Ping?

I believe I remarked that in the example
typedef struct Foo {
would be more in line with our Coding Style and majority of users.

Other than that, I have no objections and assumed you'll take it through
your arm queue.

Cheers,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v2 0/4] Allow QOM struct fields to be marked as private

[Peter Maydell]

On 23 May 2014 12:23, Andreas Färber <afaerber@suse.de> wrote:
> Am 23.05.2014 13:13, schrieb Peter Maydell:
>> Ping?
>
> I believe I remarked that in the example
> typedef struct Foo {
> would be more in line with our Coding Style and majority of users.
>
> Other than that, I have no objections and assumed you'll take it through
> your arm queue.

Oops, yes, I'd forgotten that conversation. I'll remove
the newline preceding the '{' and am happy to put this
through the target-arm queue.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 0/4] Allow QOM struct fields to be marked as private

[Paolo Bonzini]

Il 23/05/2014 13:50, Peter Maydell ha scritto:
> On 23 May 2014 12:23, Andreas Färber <afaerber@suse.de> wrote:
>> Am 23.05.2014 13:13, schrieb Peter Maydell:
>>> Ping?
>>
>> I believe I remarked that in the example
>> typedef struct Foo {
>> would be more in line with our Coding Style and majority of users.
>>
>> Other than that, I have no objections and assumed you'll take it through
>> your arm queue.
> 
> Oops, yes, I'd forgotten that conversation. I'll remove
> the newline preceding the '{' and am happy to put this
> through the target-arm queue.

Semi-serious proposal:

    #define comma_if_empty_ ,
    #define CPP_IFEMPTY(macro, t, f)      CPP_IFEMPTY1(macro, t, f)
    #define CPP_IFEMPTY1(value, t, f)     CPP_IF2(comma_if_empty_##value, t, f)
    #define CPP_IF2(comma_if_true, t, f)  CPP_IF3(comma_if_true t, f)
    #define CPP_IF3(_, v, ...) v

    #define qom_private(macro) CPP_IFEMPTY(IMPLEMENTING_##macro,, QEMU_PRIVATE_ATTR)

To be used as:

    qom_private(A9_SCU) MemoryRegion iomem;

Credit: https://plus.google.com/u/0/+LinusTorvalds/posts/9gntjh57dXt

Paolo

Re: [Qemu-devel] [PATCH v2 0/4] Allow QOM struct fields to be marked as private

[Peter Maydell]

On 23 May 2014 15:33, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 23/05/2014 13:50, Peter Maydell ha scritto:
>> On 23 May 2014 12:23, Andreas Färber <afaerber@suse.de> wrote:
>>> Am 23.05.2014 13:13, schrieb Peter Maydell:
>>>> Ping?
>>>
>>> I believe I remarked that in the example
>>> typedef struct Foo {
>>> would be more in line with our Coding Style and majority of users.
>>>
>>> Other than that, I have no objections and assumed you'll take it through
>>> your arm queue.
>>
>> Oops, yes, I'd forgotten that conversation. I'll remove
>> the newline preceding the '{' and am happy to put this
>> through the target-arm queue.
>
> Semi-serious proposal:
>
>     #define comma_if_empty_ ,
>     #define CPP_IFEMPTY(macro, t, f)      CPP_IFEMPTY1(macro, t, f)
>     #define CPP_IFEMPTY1(value, t, f)     CPP_IF2(comma_if_empty_##value, t, f)
>     #define CPP_IF2(comma_if_true, t, f)  CPP_IF3(comma_if_true t, f)
>     #define CPP_IF3(_, v, ...) v
>
>     #define qom_private(macro) CPP_IFEMPTY(IMPLEMENTING_##macro,, QEMU_PRIVATE_ATTR)
>
> To be used as:
>
>     qom_private(A9_SCU) MemoryRegion iomem;

Interesting. That means we can avoid the irritating
boilerplate in each include file to define and undefine
qom_private, at the cost of the tag on each field in the
struct being a bit longer. I'm not entirely sure which
I prefer...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 0/4] Allow QOM struct fields to be marked as private

[Paolo Bonzini]

Il 23/05/2014 18:21, Peter Maydell ha scritto:
> On 23 May 2014 15:33, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 23/05/2014 13:50, Peter Maydell ha scritto:
>>> On 23 May 2014 12:23, Andreas Färber <afaerber@suse.de> wrote:
>>>> Am 23.05.2014 13:13, schrieb Peter Maydell:
>>>>> Ping?
>>>>
>>>> I believe I remarked that in the example
>>>> typedef struct Foo {
>>>> would be more in line with our Coding Style and majority of users.
>>>>
>>>> Other than that, I have no objections and assumed you'll take it through
>>>> your arm queue.
>>>
>>> Oops, yes, I'd forgotten that conversation. I'll remove
>>> the newline preceding the '{' and am happy to put this
>>> through the target-arm queue.
>>
>> Semi-serious proposal:
>>
>>     #define comma_if_empty_ ,
>>     #define CPP_IFEMPTY(macro, t, f)      CPP_IFEMPTY1(macro, t, f)
>>     #define CPP_IFEMPTY1(value, t, f)     CPP_IF2(comma_if_empty_##value, t, f)
>>     #define CPP_IF2(comma_if_true, t, f)  CPP_IF3(comma_if_true t, f)
>>     #define CPP_IF3(_, v, ...) v
>>
>>     #define qom_private(macro) CPP_IFEMPTY(IMPLEMENTING_##macro,, QEMU_PRIVATE_ATTR)
>>
>> To be used as:
>>
>>     qom_private(A9_SCU) MemoryRegion iomem;
>
> Interesting. That means we can avoid the irritating
> boilerplate in each include file to define and undefine
> qom_private, at the cost of the tag on each field in the
> struct being a bit longer. I'm not entirely sure which
> I prefer...

You could also

#define a9_scu_private qom_private(A9_SCU)

    a9_scu_private MemoryRegion iommu;

where the difference becomes a wash.

Paolo