Patchwork [RFC,3/3] net.h: Add description fields for qdev properites

login
register
mail settings
Submitter Amit Shah
Date May 28, 2010, 9:57 a.m.
Message ID <4df39362e877c2702834d869e9098ef77dc348fb.1275040675.git.amit.shah@redhat.com>
Download mbox | patch
Permalink /patch/53878/
State New
Headers show

Comments

Amit Shah - May 28, 2010, 9:57 a.m.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 net.h |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)
Markus Armbruster - May 28, 2010, 12:50 p.m.
Amit Shah <amit.shah@redhat.com> writes:

> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  net.h |   13 ++++++++++---
>  1 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/net.h b/net.h
> index 0e9cc5d..8ad439a 100644
> --- a/net.h
> +++ b/net.h
> @@ -20,10 +20,17 @@ typedef struct NICConf {
>      VLANClientState *peer;
>  } NICConf;
>  
> +#define PROP_NET_MACADDR_DESC                   \
> +    "The MAC address for the NIC"
> +#define PROP_NET_VLAN_DESC                      \
> +    "The VLAN to associate the NIC with"
> +#define PROP_NET_NETDEV_DESC                    \
> +    "The peer net device to associate with this virtual NIC"
> +
>  #define DEFINE_NIC_PROPERTIES(_state, _conf)                            \
> -    DEFINE_PROP_MACADDR("mac",   _state, _conf.macaddr, ""),            \
> -    DEFINE_PROP_VLAN("vlan",     _state, _conf.vlan, ""),               \
> -    DEFINE_PROP_NETDEV("netdev", _state, _conf.peer, "")
> +    DEFINE_PROP_MACADDR("mac",   _state, _conf.macaddr, PROP_NET_MACADDR_DESC),\
> +    DEFINE_PROP_VLAN("vlan",     _state, _conf.vlan, PROP_NET_VLAN_DESC),     \
> +    DEFINE_PROP_NETDEV("netdev", _state, _conf.peer, PROP_NET_NETDEV_DESC)
>  
>  /* VLANs support */

Why the macros?
Amit Shah - May 28, 2010, 12:59 p.m.
On (Fri) May 28 2010 [14:50:46], Markus Armbruster wrote:
> Amit Shah <amit.shah@redhat.com> writes:
> 
> > Signed-off-by: Amit Shah <amit.shah@redhat.com>
> > ---
> >  net.h |   13 ++++++++++---
> >  1 files changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/net.h b/net.h
> > index 0e9cc5d..8ad439a 100644
> > --- a/net.h
> > +++ b/net.h
> > @@ -20,10 +20,17 @@ typedef struct NICConf {
> >      VLANClientState *peer;
> >  } NICConf;
> >  
> > +#define PROP_NET_MACADDR_DESC                   \
> > +    "The MAC address for the NIC"
> > +#define PROP_NET_VLAN_DESC                      \
> > +    "The VLAN to associate the NIC with"
> > +#define PROP_NET_NETDEV_DESC                    \
> > +    "The peer net device to associate with this virtual NIC"
> > +
> >  #define DEFINE_NIC_PROPERTIES(_state, _conf)                            \
> > -    DEFINE_PROP_MACADDR("mac",   _state, _conf.macaddr, ""),            \
> > -    DEFINE_PROP_VLAN("vlan",     _state, _conf.vlan, ""),               \
> > -    DEFINE_PROP_NETDEV("netdev", _state, _conf.peer, "")
> > +    DEFINE_PROP_MACADDR("mac",   _state, _conf.macaddr, PROP_NET_MACADDR_DESC),\
> > +    DEFINE_PROP_VLAN("vlan",     _state, _conf.vlan, PROP_NET_VLAN_DESC),     \
> > +    DEFINE_PROP_NETDEV("netdev", _state, _conf.peer, PROP_NET_NETDEV_DESC)
> >  
> >  /* VLANs support */
> 
> Why the macros?

Just seems cleaner; big strings can become an eyesore in such places.
What do you think?

		Amit
Markus Armbruster - May 28, 2010, 1:45 p.m.
Amit Shah <amit.shah@redhat.com> writes:

> On (Fri) May 28 2010 [14:50:46], Markus Armbruster wrote:
>> Amit Shah <amit.shah@redhat.com> writes:
>> 
>> > Signed-off-by: Amit Shah <amit.shah@redhat.com>
>> > ---
>> >  net.h |   13 ++++++++++---
>> >  1 files changed, 10 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/net.h b/net.h
>> > index 0e9cc5d..8ad439a 100644
>> > --- a/net.h
>> > +++ b/net.h
>> > @@ -20,10 +20,17 @@ typedef struct NICConf {
>> >      VLANClientState *peer;
>> >  } NICConf;
>> >  
>> > +#define PROP_NET_MACADDR_DESC                   \
>> > +    "The MAC address for the NIC"
>> > +#define PROP_NET_VLAN_DESC                      \
>> > +    "The VLAN to associate the NIC with"
>> > +#define PROP_NET_NETDEV_DESC                    \
>> > +    "The peer net device to associate with this virtual NIC"
>> > +
>> >  #define DEFINE_NIC_PROPERTIES(_state, _conf)                            \
>> > -    DEFINE_PROP_MACADDR("mac",   _state, _conf.macaddr, ""),            \
>> > -    DEFINE_PROP_VLAN("vlan",     _state, _conf.vlan, ""),               \
>> > -    DEFINE_PROP_NETDEV("netdev", _state, _conf.peer, "")
>> > +    DEFINE_PROP_MACADDR("mac",   _state, _conf.macaddr, PROP_NET_MACADDR_DESC),\
>> > +    DEFINE_PROP_VLAN("vlan",     _state, _conf.vlan, PROP_NET_VLAN_DESC),     \
>> > +    DEFINE_PROP_NETDEV("netdev", _state, _conf.peer, PROP_NET_NETDEV_DESC)
>> >  
>> >  /* VLANs support */
>> 
>> Why the macros?
>
> Just seems cleaner; big strings can become an eyesore in such places.
> What do you think?

I'd say the macros are a worse eyesore :)

Compare:

#define DEFINE_NIC_PROPERTIES(_state, _conf)                            \
    DEFINE_PROP_MACADDR("mac",   _state, _conf.macaddr, PROP_NET_MACADDR_DESC),\
    DEFINE_PROP_VLAN("vlan",     _state, _conf.vlan, PROP_NET_VLAN_DESC),     \
    DEFINE_PROP_NETDEV("netdev", _state, _conf.peer, PROP_NET_NETDEV_DESC)

#define DEFINE_NIC_PROPERTIES(_state, _conf)                            \
    DEFINE_PROP_MACADDR("mac",   _state, _conf.macaddr,			\
        "The MAC address for the NIC"),					\
    DEFINE_PROP_VLAN("vlan",     _state, _conf.vlan,			\
        "The VLAN to associate the NIC with"),				\
    DEFINE_PROP_NETDEV("netdev", _state, _conf.peer,			\
        "The peer net device to associate with this virtual NIC")

This way, the description string does double-duty as documentation of
the property definition.

Patch

diff --git a/net.h b/net.h
index 0e9cc5d..8ad439a 100644
--- a/net.h
+++ b/net.h
@@ -20,10 +20,17 @@  typedef struct NICConf {
     VLANClientState *peer;
 } NICConf;
 
+#define PROP_NET_MACADDR_DESC                   \
+    "The MAC address for the NIC"
+#define PROP_NET_VLAN_DESC                      \
+    "The VLAN to associate the NIC with"
+#define PROP_NET_NETDEV_DESC                    \
+    "The peer net device to associate with this virtual NIC"
+
 #define DEFINE_NIC_PROPERTIES(_state, _conf)                            \
-    DEFINE_PROP_MACADDR("mac",   _state, _conf.macaddr, ""),            \
-    DEFINE_PROP_VLAN("vlan",     _state, _conf.vlan, ""),               \
-    DEFINE_PROP_NETDEV("netdev", _state, _conf.peer, "")
+    DEFINE_PROP_MACADDR("mac",   _state, _conf.macaddr, PROP_NET_MACADDR_DESC),\
+    DEFINE_PROP_VLAN("vlan",     _state, _conf.vlan, PROP_NET_VLAN_DESC),     \
+    DEFINE_PROP_NETDEV("netdev", _state, _conf.peer, PROP_NET_NETDEV_DESC)
 
 /* VLANs support */