> +/* Deletes 'node' from 'sh'. Neither the node's key nor its value is freed;
> + * instead, ownership is transferred to the caller. Returns the node's key.
> */
This should say from 'smap'
> +char *
> +smap_steal(struct smap *smap, struct smap_node *node)
> +{
> + char *key = node->key;
> +
> + hmap_remove(&smap->map, &node->node);
> + free(node);
> + return key;
> +}
> +
I think the definition of this function is a bit misleading in a way
that could cause memory leaks if someone isn't careful. Basically,
the only way to free node->value is to keep a pointer to it before
calling this function, otherwise node (containing the only pointer to
node->value) is dead. This seems fine to me, but it's strange that
you have to deal with value in this way, while key is helpfully
returned by the function. I'm afraid callers are going to forget to
free value entirely causing leaks.
I can think of two possibly better implementations:
1) Return Nothing. This would make the handling of key and value
consistent, reminding callers that both need to be dealt with.
2) Change the prototype to:
void
smap_steal(struct smap *smap, struct smap *node,
char **key, char **value);
If key is NULL, node->key is freed, otherwise node->key is put into
key. Same with value.
What do you think?
Ethan
> /* Removes all key-value pairs from 'smap'. */
> void
> smap_clear(struct smap *smap)
> diff --git a/lib/smap.h b/lib/smap.h
> index 8510a37..13eec1c 100644
> --- a/lib/smap.h
> +++ b/lib/smap.h
> @@ -48,7 +48,8 @@ void smap_add_format(struct smap *, const char *key, const
> char *, ...)
> void smap_replace(struct smap *, const char *, const char *);
>
> void smap_remove(struct smap *, const char *);
> -void smap_remove_node(struct smap *smap, struct smap_node *);
> +void smap_remove_node(struct smap *, struct smap_node *);
> +char *smap_steal(struct smap *, struct smap_node *);
> void smap_clear(struct smap *);
>
> const char *smap_get(const struct smap *, const char *);
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
