On Wed, Jun 13, 2012 at 06:12:17PM -0700, Ethan Jackson wrote:
> String to string maps are used all over the Open vSwitch database.
> Before this patch, they were implemented in the idl as parallel
> string arrays. This strategy has proven a bit cumbersome. With
> this patch, string to strong maps are implemented using the smap
s/strong/string/ here.
The interface to the ovsrec_*_init() functions seems odd. All of them
take a pointer to a generic row type and then internally cast it. Why
don't they take the desired row type directly? As far as I can see, all
of the callers actually have to go to extra trouble to get the
ovsdb_idl_row member:
vswitchd/bridge.c: ovsrec_interface_init(&br->synth_local_iface.header_);
vswitchd/bridge.c: ovsrec_port_init(&br->synth_local_port.header_);
vswitchd/bridge.c: ovsrec_interface_init(&iface->header_);
vswitchd/bridge.c: ovsrec_port_init(&iface->header_);
In ovsdb-idlc.in, this bit of the generated code looks wrong to me:
SMAP_FOR_EACH (node, smap) {
for (i = 0; i < datum.n; i++) {
datum.keys[i].string = xstrdup(node->key);
datum.values[i].string = xstrdup(node->value);
}
}
I'd expect something more like this?
i = 0;
SMAP_FOR_EACH (node, smap) {
datum.keys[i].string = xstrdup(node->key);
datum.values[i].string = xstrdup(node->value);
i++;
}
Around the same place,
memset(&datum, 0, sizeof datum);
is probably better expressed as:
ovsdb_datum_init_empty(&datum);
Can port_configure_stp() use smap_get_bool()?
In bridge_configure_remotes(), I think that the default for
disable_in_band (what a terrible name for an option) should be false,
not true.
synthesize_splinter_port() now has a call to ovsrec_port_init() and an
explicit call of smap_init(&port->other_config). Doesn't the former
keep the latter from being necessary?
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
