[libvirt] [PATCH] nwfilter: Make entries in a int-2-string map more readable
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@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 };
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Eric Blake <eblake@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
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@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 :|
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@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
"Daniel P. Berrange" <berrange@redhat.com> wrote on 03/30/2010 11:55:33 AM: +
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 :|
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
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@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@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Eric Blake -
Stefan Berger