> > + /* Remember the names of all address sets currently in
> > expr_address_sets
> > + * so we can detect address sets that have been deleted. */
> > + struct sset cur_address_sets = SSET_INITIALIZER(&cur_address_sets);
> >
> This sset is not an address_set, but address_set names (or keys).
> How about renaming this variable to cur_address_set_names ?
I think I'm agree with Flavio.
> + if (!address_sets_match(addr_set, addr_set_rec)) {> +
> expr_macros_remove(&expr_address_sets, addr_set_rec->name);> +
> address_set_destroy(addr_set);> + addr_set = NULL;> +
> create_set = true;> + }
nit: is 'addr_set = NULL;' necessary here, seems it will be assigned later:
> + addr_set = xzalloc(sizeof *addr_set);
> + addr_set->n_addresses = addr_set_rec->n_addresses;
> + if (addr_set_rec->n_addresses) {
> + addr_set->addresses = xmalloc(addr_set_rec->n_addresses
> + * sizeof
> addr_set->addresses[0]);
> + size_t i;
> + for (i = 0; i < addr_set_rec->n_addresses; i++) {
> + addr_set->addresses[i] =
> xstrdup(addr_set_rec->addresses[i]);
> + }
> + }
> + shash_add(&local_address_sets, addr_set_rec->name, addr_set);
> +
> + expr_macros_add(&expr_address_sets, addr_set_rec->name,
> + (const char * const *) addr_set->addresses,
> + addr_set->n_addresses);
So it's possible to create an empty Address_Set, right? What shall we
do with an empty Address_Set?
> + SSET_FOR_EACH_SAFE (cur_node, next_node, &cur_address_sets) {
> + expr_macros_remove(&expr_address_sets, cur_node);
> +
> + struct address_set *addr_set
> + = shash_find_and_delete(&local_address_sets, cur_node);
> + address_set_destroy(addr_set);
> +
> + struct sset_node *sset_node = SSET_NODE_FROM_NAME(cur_node);
> + sset_delete(&cur_address_sets, sset_node);
> + }
> +
> + sset_destroy(&cur_address_sets);
I think you don't need manually call sset_delete, sset_destroy will do
it for you.
> + SHASH_FOR_EACH_SAFE (node, next, &sb_address_sets) {
> + sbrec_address_set_delete(node->data);
> + shash_delete(&sb_address_sets, node);
> + }
> + shash_destroy(&sb_address_sets);
The same, you don't need manually call shash_delete, shash_destroy
will do it for you.
Thanks and have a nice day! :)
Zong Kai, LI
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
