On 09/01/2015 04:35 PM, Ben Pfaff wrote: > Reviewing the ovn-controller code I started to notice a common pattern: > > struct smap ext_ids = SMAP_INITIALIZER(&ext_ids); > smap_add(&ext_ids, "ovn-patch-port", network); > ovsrec_port_set_external_ids(port, &ext_ids); > smap_destroy(&ext_ids); > > This seemed like a bit too much code for something as simple as > initializing an smap with a single key-value pair. This commit allows the > code to be reduced to just: > > const struct smap ids = SMAP_INIT1(&ids, "ovn-patch-port", network); > ovsrec_port_set_external_ids(port, &ids); > > This new form also eliminates multiple memory allocation and free > operations, but I doubt that has any real effect on performance; > the primary goal here is code readability.
I like that the code is so much shorter. However, I'm not sure it's any more readable to me. The main thing that bothers me is that smap_destroy() is now only used *some* of the time, where it used to be nice and consistent. I wonder if there's something we could do with the macro name that would make it more clear that it doesn't need to be destroyed. SMAP_LOCAL_INIT1? I'm just nit picking, though ... I think it's still an overall net benefit. Acked-by: Russell Bryant <rbry...@redhat.com> -- Russell Bryant _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
