10:26 a.m.
A cosmetic change that makes the entries in the int-2-string maps look
more readable. Add some missing entries.
Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com>
---
src/conf/nwfilter_conf.c | 97 +++++++++++++----------------------------------
1 file changed, 28 insertions(+), 69 deletions(-)
Index: libvirt-acl/src/conf/nwfilter_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -105,6 +105,9 @@ struct int_map {
const char *val;
};
+#define INTMAP_ENTRY(ATT, VAL) { .attr = ATT, .val = VAL }
+#define INTMAP_ENTRY_LAST { .val = NULL }
+
/*
* only one filter update allowed
@@ -388,18 +391,10 @@ struct _virXMLAttr2Struct
static const struct int_map macProtoMap[] = {
- {
- .attr = ETHERTYPE_ARP,
- .val = "arp",
- }, {
- .attr = ETHERTYPE_IP,
- .val = "ipv4",
- }, {
- .attr = ETHERTYPE_IPV6,
- .val = "ipv6",
- }, {
- .val = NULL,
- }
+ INTMAP_ENTRY(ETHERTYPE_ARP , "arp"),
+ INTMAP_ENTRY(ETHERTYPE_IP , "ipv4"),
+ INTMAP_ENTRY(ETHERTYPE_IPV6, "ipv6"),
+ INTMAP_ENTRY_LAST
};
@@ -486,36 +481,16 @@ checkMACMask(enum attrDatatype datatype
* supported arp opcode -- see 'ebtables -h arp' for the naming
*/
static const struct int_map arpOpcodeMap[] = {
- {
- .attr = 1,
- .val = "Request",
- } , {
- .attr = 2,
- .val = "Reply",
- } , {
- .attr = 3,
- .val = "Request_Reverse",
- } , {
- .attr = 4,
- .val = "Reply_Reverse",
- } , {
- .attr = 5,
- .val = "DRARP_Request",
- } , {
- .attr = 6,
- .val = "DRARP_Reply",
- } , {
- .attr = 7,
- .val = "DRARP_Error",
- } , {
- .attr = 8,
- .val = "InARP_Request",
- } , {
- .attr = 9,
- .val = "ARP_NAK",
- } , {
- .val = NULL,
- }
+ INTMAP_ENTRY(1, "Request"),
+ INTMAP_ENTRY(2, "Reply"),
+ INTMAP_ENTRY(3, "Request_Reverse"),
+ INTMAP_ENTRY(4, "Reply_Reverse"),
+ INTMAP_ENTRY(5, "DRARP_Request"),
+ INTMAP_ENTRY(6, "DRARP_Reply"),
+ INTMAP_ENTRY(7, "DRARP_Error"),
+ INTMAP_ENTRY(8, "InARP_Request"),
+ INTMAP_ENTRY(9, "ARP_NAK"),
+ INTMAP_ENTRY_LAST
};
@@ -562,37 +537,21 @@ arpOpcodeFormatter(virBufferPtr buf,
static const struct int_map ipProtoMap[] = {
- {
- .attr = IPPROTO_TCP,
- .val = "tcp",
- } , {
- .attr = IPPROTO_UDP,
- .val = "udp",
+ INTMAP_ENTRY(IPPROTO_TCP, "tcp"),
+ INTMAP_ENTRY(IPPROTO_UDP, "udp"),
#ifdef IPPROTO_UDPLITE
- } , {
- .attr = IPPROTO_UDPLITE,
- .val = "udplite",
+ INTMAP_ENTRY(IPPROTO_UDPLITE, "udplite"),
#endif
- } , {
- .attr = IPPROTO_ESP,
- .val = "esp",
- } , {
- .attr = IPPROTO_AH,
- .val = "ah",
- } , {
- .attr = IPPROTO_ICMP,
- .val = "icmp",
- } , {
- .attr = IPPROTO_IGMP,
- .val = "igmp",
+ INTMAP_ENTRY(IPPROTO_ESP, "esp"),
+ INTMAP_ENTRY(IPPROTO_AH, "ah"),
+ INTMAP_ENTRY(IPPROTO_ICMP, "icmp"),
+ INTMAP_ENTRY(IPPROTO_IGMP, "igmp"),
#ifdef IPPROTO_SCTP
- } , {
- .attr = IPPROTO_SCTP,
- .val = "sctp",
+ INTMAP_ENTRY(IPPROTO_SCTP, "sctp"),
#endif
- } , {
- .val = NULL,
- }
+ INTMAP_ENTRY(IPPROTO_IPV6, "ipv6"),
+ INTMAP_ENTRY(IPPROTO_ICMPV6, "icmpv6"),
+ INTMAP_ENTRY_LAST
};
10:34 a.m.
New subject: [libvirt] [PATCH] nwfilter: Make entries in a int-2-string map more readable
On 03/30/2010 09:26 AM, Stefan Berger wrote:
A cosmetic change that makes the entries in the int-2-string maps
look
more readable.
That's for sure :)
Add some missing entries.
Is it worth mentioning which ones in the commit message? Other than
that, ACK.
#endif
- } , {
- .val = NULL,
- }
+ INTMAP_ENTRY(IPPROTO_IPV6, "ipv6"),
+ INTMAP_ENTRY(IPPROTO_ICMPV6, "icmpv6"),
+ INTMAP_ENTRY_LAST
ipv6 and icmpv6 are the only new ones, right?
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
10:40 a.m.
New subject: [libvirt] [PATCH] nwfilter: Make entries in a int-2-string map more readable
Eric Blake <eblake(a)redhat.com> wrote on 03/30/2010 11:34:41 AM:
On 03/30/2010 09:26 AM, Stefan Berger wrote:
> A cosmetic change that makes the entries in the int-2-string maps look
> more readable.
That's for sure :)
> Add some missing entries.
Is it worth mentioning which ones in the commit message? Other than
that, ACK.
> #endif
> - } , {
> - .val = NULL,
> - }
> + INTMAP_ENTRY(IPPROTO_IPV6, "ipv6"),
> + INTMAP_ENTRY(IPPROTO_ICMPV6, "icmpv6"),
> + INTMAP_ENTRY_LAST
ipv6 and icmpv6 are the only new ones, right?
Correct.
Stefan
10:55 a.m.
On Tue, Mar 30, 2010 at 11:26:47AM -0400, Stefan Berger wrote:
A cosmetic change that makes the entries in the int-2-string maps
look
more readable. Add some missing entries.
Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com>
---
src/conf/nwfilter_conf.c | 97 +++++++++++++----------------------------------
1 file changed, 28 insertions(+), 69 deletions(-)
Index: libvirt-acl/src/conf/nwfilter_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -105,6 +105,9 @@ struct int_map {
const char *val;
};
+#define INTMAP_ENTRY(ATT, VAL) { .attr = ATT, .val = VAL }
+#define INTMAP_ENTRY_LAST { .val = NULL }
+
/*
* only one filter update allowed
@@ -388,18 +391,10 @@ struct _virXMLAttr2Struct
static const struct int_map macProtoMap[] = {
- {
- .attr = ETHERTYPE_ARP,
- .val = "arp",
- }, {
- .attr = ETHERTYPE_IP,
- .val = "ipv4",
- }, {
- .attr = ETHERTYPE_IPV6,
- .val = "ipv6",
- }, {
- .val = NULL,
- }
+ INTMAP_ENTRY(ETHERTYPE_ARP , "arp"),
+ INTMAP_ENTRY(ETHERTYPE_IP , "ipv4"),
+ INTMAP_ENTRY(ETHERTYPE_IPV6, "ipv6"),
+ INTMAP_ENTRY_LAST
};
This should all really be replaced with a standard call to VIR_ENUM_DECL +
VIR_ENUM_IMPL which do compile time validation that you have the correct
number of strings to match the enum, and allow O(1) int to string conversion,
though string to int is still O(n).
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
3:59 p.m.
"Daniel P. Berrange" <berrange(a)redhat.com> wrote on 03/30/2010 11:55:33
AM:
Please respond to "Daniel P. Berrange"
On Tue, Mar 30, 2010 at 11:26:47AM -0400, Stefan Berger wrote:
> A cosmetic change that makes the entries in the int-2-string maps look
> more readable. Add some missing entries.
>
> Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com>
>
>
> ---
> src/conf/nwfilter_conf.c | 97 ++++++++++++
+----------------------------------
> 1 file changed, 28 insertions(+), 69 deletions(-)
>
> Index: libvirt-acl/src/conf/nwfilter_conf.c
> ===================================================================
> --- libvirt-acl.orig/src/conf/nwfilter_conf.c
> +++ libvirt-acl/src/conf/nwfilter_conf.c
> @@ -105,6 +105,9 @@ struct int_map {
> const char *val;
> };
>
> +#define INTMAP_ENTRY(ATT, VAL) { .attr = ATT, .val = VAL }
> +#define INTMAP_ENTRY_LAST { .val = NULL }
> +
>
> /*
> * only one filter update allowed
> @@ -388,18 +391,10 @@ struct _virXMLAttr2Struct
>
>
> static const struct int_map macProtoMap[] = {
> - {
> - .attr = ETHERTYPE_ARP,
> - .val = "arp",
> - }, {
> - .attr = ETHERTYPE_IP,
> - .val = "ipv4",
> - }, {
> - .attr = ETHERTYPE_IPV6,
> - .val = "ipv6",
> - }, {
> - .val = NULL,
> - }
> + INTMAP_ENTRY(ETHERTYPE_ARP , "arp"),
> + INTMAP_ENTRY(ETHERTYPE_IP , "ipv4"),
> + INTMAP_ENTRY(ETHERTYPE_IPV6, "ipv6"),
> + INTMAP_ENTRY_LAST
> };
This should all really be replaced with a standard call to VIR_ENUM_DECL
+
VIR_ENUM_IMPL which do compile time validation that you have the
correct
number of strings to match the enum, and allow O(1) int to string
conversion,
though string to int is still O(n).
ETHERTYPE_ARP has value 0x806. From my understanding this wouldn't fit the
VIR_ENUM_DECL macros. O(log(n)) is the best we could do for int lookups...
but I doubt it'd be worth the effort for the small maps.
Regards,
Stefan
Regards,
Daniel
--
|: Red Hat, Engineering, London -o-
http://people.redhat.com/berrange/:|
|: http://libvirt.org -o- http://virt-manager.org -o-
http://deltacloud.org:|
|: http://autobuild.org -o-
http://search.cpan.org/~danberr/:|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742
7D3B
9505 :|
4:39 p.m.
On 03/30/2010 09:55 AM, Daniel P. Berrange wrote:
> + INTMAP_ENTRY(ETHERTYPE_ARP , "arp"),
> + INTMAP_ENTRY(ETHERTYPE_IP , "ipv4"),
> + INTMAP_ENTRY(ETHERTYPE_IPV6, "ipv6"),
> + INTMAP_ENTRY_LAST
> };
This should all really be replaced with a standard call to VIR_ENUM_DECL +
VIR_ENUM_IMPL which do compile time validation that you have the correct
number of strings to match the enum, and allow O(1) int to string conversion,
though string to int is still O(n).
If I understand util.h right, those macros only work for enumerators
that are consecutive and which start at 0. But Stefan is mapping
external constants, with no guarantee that they are consecutive nor that
they start at 0. The only way to use VIR_ENUM_DECL is to write our own
wrapper enums, naming them something like VIR_ETHERTYPE_ARP; but I don't
see what it would buy us, because then we would have to convert from our
wrapper to the known external constants.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
10:55 a.m.
On Tue, Mar 30, 2010 at 11:26:47AM -0400, Stefan Berger wrote:
A cosmetic change that makes the entries in the int-2-string maps
look
more readable. Add some missing entries.
Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com>
---
src/conf/nwfilter_conf.c | 97 +++++++++++++----------------------------------
1 file changed, 28 insertions(+), 69 deletions(-)
Index: libvirt-acl/src/conf/nwfilter_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_conf.c
+++ libvirt-acl/src/conf/nwfilter_conf.c
@@ -105,6 +105,9 @@ struct int_map {
const char *val;
};
+#define INTMAP_ENTRY(ATT, VAL) { .attr = ATT, .val = VAL }
+#define INTMAP_ENTRY_LAST { .val = NULL }
+
makes the code easier to read,
ACK
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
5386
days inactive
5386
days old
6 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Eric Blake -
Stefan Berger