Thanks for the fix, looks good to me.
Reviewed-by: Yifeng Sun <[email protected]> On Fri, Mar 23, 2018 at 3:46 PM, Ben Pfaff <[email protected]> wrote: > Until now, every time the JSON parser added an object member, it made an > extra copy of the member name and then freed the original copy. This is > wasteful, so this commit eliminates the extra copy. > > Signed-off-by: Ben Pfaff <[email protected]> > --- > include/openvswitch/json.h | 1 + > include/openvswitch/shash.h | 1 + > lib/json.c | 9 +++++++-- > lib/shash.c | 23 +++++++++++++++++++++++ > 4 files changed, 32 insertions(+), 2 deletions(-) > > diff --git a/include/openvswitch/json.h b/include/openvswitch/json.h > index 61b9a02cfc19..bcf6a27826ae 100644 > --- a/include/openvswitch/json.h > +++ b/include/openvswitch/json.h > @@ -91,6 +91,7 @@ struct json *json_array_create_3(struct json *, struct > json *, struct json *); > > struct json *json_object_create(void); > void json_object_put(struct json *, const char *name, struct json *value); > +void json_object_put_nocopy(struct json *, char *name, struct json > *value); > void json_object_put_string(struct json *, > const char *name, const char *value); > void json_object_put_format(struct json *, > diff --git a/include/openvswitch/shash.h b/include/openvswitch/shash.h > index afc482364f24..c249e13e1f28 100644 > --- a/include/openvswitch/shash.h > +++ b/include/openvswitch/shash.h > @@ -62,6 +62,7 @@ struct shash_node *shash_add_nocopy(struct shash *, char > *, const void *); > bool shash_add_once(struct shash *, const char *, const void *); > void shash_add_assert(struct shash *, const char *, const void *); > void *shash_replace(struct shash *, const char *, const void *data); > +void *shash_replace_nocopy(struct shash *, char *name, const void *data); > void shash_delete(struct shash *, struct shash_node *); > char *shash_steal(struct shash *, struct shash_node *); > struct shash_node *shash_find(const struct shash *, const char *); > diff --git a/lib/json.c b/lib/json.c > index 07ca87b2130f..603fd1df8b6a 100644 > --- a/lib/json.c > +++ b/lib/json.c > @@ -279,6 +279,12 @@ json_object_put(struct json *json, const char *name, > struct json *value) > json_destroy(shash_replace(json->u.object, name, value)); > } > > +void > +json_object_put_nocopy(struct json *json, char *name, struct json *value) > +{ > + json_destroy(shash_replace_nocopy(json->u.object, name, value)); > +} > + > void > json_object_put_string(struct json *json, const char *name, const char > *value) > { > @@ -1217,8 +1223,7 @@ json_parser_put_value(struct json_parser *p, struct > json *value) > { > struct json_parser_node *node = json_parser_top(p); > if (node->json->type == JSON_OBJECT) { > - json_object_put(node->json, p->member_name, value); > - free(p->member_name); > + json_object_put_nocopy(node->json, p->member_name, value); > p->member_name = NULL; > } else if (node->json->type == JSON_ARRAY) { > json_array_add(node->json, value); > diff --git a/lib/shash.c b/lib/shash.c > index 318a30ffc0f0..a8433629ab84 100644 > --- a/lib/shash.c > +++ b/lib/shash.c > @@ -166,6 +166,29 @@ shash_replace(struct shash *sh, const char *name, > const void *data) > } > } > > +/* Searches for 'name' in 'sh'. If it does not already exist, adds it > along > + * with 'data' and returns NULL. If it does already exist, replaces its > data > + * by 'data' and returns the data that it formerly contained. > + * > + * Takes ownership of 'name'. */ > +void * > +shash_replace_nocopy(struct shash *sh, char *name, const void *data) > +{ > + size_t hash = hash_name(name); > + struct shash_node *node; > + > + node = shash_find__(sh, name, strlen(name), hash); > + if (!node) { > + shash_add_nocopy__(sh, name, data, hash); > + return NULL; > + } else { > + free(name); > + void *old_data = node->data; > + node->data = CONST_CAST(void *, data); > + return old_data; > + } > +} > + > /* Deletes 'node' from 'sh' and frees the node's name. The caller is > still > * responsible for freeing the node's data, if necessary. */ > void > -- > 2.16.1 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
