All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: "Andreas Färber" <afaerber@suse.de>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	qemu-devel@nongnu.org, Anthony Liguori <anthony@codemonkey.ws>,
	Igor Mammedov <imammedo@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] qom: helper macro for adding read-only properties
Date: Mon, 16 Sep 2013 19:07:21 +0300	[thread overview]
Message-ID: <20130916160721.GA4386@redhat.com> (raw)
In-Reply-To: <5236A5ED.3000804@suse.de>

On Mon, Sep 16, 2013 at 08:32:13AM +0200, Andreas Färber wrote:
> Am 15.09.2013 19:23, schrieb Michael S. Tsirkin:
> > Add a helper macro for adding read-only properties, that works in the
> > common case where the value is a constant.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > 
> > I'm using this patch in my acpi work - any objections
> > to applying it on my tree?
> 
> Actually yes: Apart from the clang issues raised and the disturbing
> upper-casing of arguments,

BTW there are no upper casing of arguments:
internal variable names in macros must include
macro name otherwise you get hard to debug collisions:

#define SQUARE_ME(x) \
	do { \
		int var = x; \
		x = var * x; \
	} while (0) \

int v = 2;
SQUARE_ME(v);

// v is 4

int var = 2;
SQUARE_ME(var);

// BUG: var is still 2


> this is hardcoding "int" type and NULL errp,
> so I don't think it deserves to live in object.h as is. I do agree that
> we could use more helper functions to deal with dynamic properties.
> 
> So what about taking bool/string property helpers as example and putting
> intX_t getters into object.c, using a passed-through opaque argument to
> obtain the value? We could then have real object_property_add_int32()
> etc. functions using the appropriate type name, with field/value pointer
> and Error** arguments. A pointer can be assumed to hold up to uint32_t
> values or, to keep the API more general, use a local static const
> variable for non-field values.
> 
> It does touch on the issue I brought up on a KVM call a couple weeks ago
> of how dynamic and static properties are supposed to relate. I
> personally welcome making dynamic properties more easy to deal with; an
> alternative might be to extend qdev-properties.c with
> DEFINE_PROP_READONLY_UINT32() etc. CC'ing Igor, who has dealt with
> dynamic-vs.-static properties for X86CPU.
> 
> Regards,
> Andreas

OK this makes you happy?
All calls pass NULL as Error so I omitted
it for now.
We can add later if anyone has a use for it.


diff --git a/include/qom/object.h b/include/qom/object.h
index 1a7b71a..90dce59 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -795,6 +795,27 @@ void object_property_add(Object *obj, const char *name, const char *type,
 void object_property_del(Object *obj, const char *name, struct Error **errp);
 
 /**
+ * object_property_add_uint8_t_ptr:
+ * object_property_add_uint16_t_ptr:
+ * object_property_add_uint32_t_ptr:
+ * object_property_add_uint64_t_ptr:
+ * @obj: the object to add a property to
+ * @name: the name of the property
+ * @v: pointer to value
+ *
+ * Add an integer property in memory.  This function will add a
+ * property of the appropriate type.
+ */
+void object_property_add_uint8_t_ptr(Object *obj, const char *name,
+                                     const uint8_t *v);
+void object_property_add_uint16_t_ptr(Object *obj, const char *name,
+                                      const uint16_t *v);
+void object_property_add_uint32_t_ptr(Object *obj, const char *name,
+                                      const uint32_t *v);
+void object_property_add_uint64_t_ptr(Object *obj, const char *name,
+                                      const uint64_t *v);
+
+/**
  * object_property_find:
  * @obj: the object
  * @name: the name of the property

      parent reply	other threads:[~2013-09-16 16:05 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-15 17:23 [Qemu-devel] [PATCH] qom: helper macro for adding read-only properties Michael S. Tsirkin
2013-09-15 17:54 ` Peter Maydell
2013-09-15 20:06   ` Michael S. Tsirkin
2013-09-16  6:32 ` Andreas Färber
2013-09-16 12:33   ` Michael S. Tsirkin
2013-09-16 15:24     ` Andreas Färber
2013-09-16 15:41       ` Paolo Bonzini
2013-09-16 15:48       ` Michael S. Tsirkin
2013-09-16 15:56         ` Paolo Bonzini
2013-09-16 16:01           ` Michael S. Tsirkin
2013-09-16 16:07             ` Paolo Bonzini
2013-09-16 16:51               ` Michael S. Tsirkin
2013-09-16 17:03                 ` Paolo Bonzini
2013-09-16 16:07   ` Michael S. Tsirkin [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130916160721.GA4386@redhat.com \
    --to=mst@redhat.com \
    --cc=afaerber@suse.de \
    --cc=anthony@codemonkey.ws \
    --cc=imammedo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.