Thanks for the change. I also find that this new function can be applied in another place:
diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c index 1e0c20486e38..f9e4e529e32e 100644 --- a/ovsdb/ovsdb-server.c +++ b/ovsdb/ovsdb-server.c @@ -517,7 +517,7 @@ open_db(struct server_config *config, const char *filename) db_error = ovsdb_file_open(db->filename, false, &db->db, &db->file); if (db_error) { - error = ovsdb_error_to_string(db_error); + error = ovsdb_error_to_string_free(db_error); } else if (!ovsdb_jsonrpc_server_add_db(config->jsonrpc, db->db)) { error = xasprintf("%s: duplicate database name", db->db->schema->name); } else { @@ -525,7 +525,6 @@ open_db(struct server_config *config, const char *filename) return NULL; } - ovsdb_error_destroy(db_error); close_db(db); return error; } Tested-by: Yifeng Sun <pkusunyif...@gmail.com> Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Wed, Dec 13, 2017 at 10:04 AM, Ben Pfaff <b...@ovn.org> wrote: > This allows slight code simplifications across the tree. > > Signed-off-by: Ben Pfaff <b...@ovn.org> > --- > lib/ovsdb-data.c | 5 ++--- > lib/ovsdb-error.c | 29 +++++++++++++++++++++++------ > lib/ovsdb-error.h | 3 ++- > lib/ovsdb-idl.c | 9 +++------ > ovsdb/file.c | 7 ++----- > ovsdb/ovsdb-server.c | 6 ++---- > tests/test-ovsdb.c | 18 ++++++------------ > 7 files changed, 40 insertions(+), 37 deletions(-) > > diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c > index 5d560fd98b24..3ddf5f5bd539 100644 > --- a/lib/ovsdb-data.c > +++ b/lib/ovsdb-data.c > @@ -1,4 +1,4 @@ > -/* Copyright (c) 2009, 2010, 2011, 2012, 2014, 2016 Nicira, Inc. > +/* Copyright (c) 2009, 2010, 2011, 2012, 2014, 2016, 2017 Nicira, Inc. > * > * Licensed under the Apache License, Version 2.0 (the "License"); > * you may not use this file except in compliance with the License. > @@ -664,8 +664,7 @@ ovsdb_atom_from_string(union ovsdb_atom *atom, > free(*range_end_atom); > *range_end_atom = NULL; > } > - msg = ovsdb_error_to_string(error); > - ovsdb_error_destroy(error); > + msg = ovsdb_error_to_string_free(error); > } > return msg; > } > diff --git a/lib/ovsdb-error.c b/lib/ovsdb-error.c > index d8161e6d7b9a..9b1af68c6ca3 100644 > --- a/lib/ovsdb-error.c > +++ b/lib/ovsdb-error.c > @@ -1,4 +1,4 @@ > -/* Copyright (c) 2009, 2010, 2011, 2012, 2016 Nicira, Inc. > +/* Copyright (c) 2009, 2010, 2011, 2012, 2016, 2017 Nicira, Inc. > * > * Licensed under the Apache License, Version 2.0 (the "License"); > * you may not use this file except in compliance with the License. > @@ -153,11 +153,9 @@ ovsdb_internal_error(struct ovsdb_error *inner_error, > ds_put_format(&ds, " (%s %s)", program_name, VERSION); > > if (inner_error) { > - char *s = ovsdb_error_to_string(inner_error); > + char *s = ovsdb_error_to_string_free(inner_error); > ds_put_format(&ds, " (generated from: %s)", s); > free(s); > - > - ovsdb_error_destroy(inner_error); > } > > error = ovsdb_error("internal error", "%s", ds_cstr(&ds)); > @@ -223,6 +221,8 @@ ovsdb_error_to_json(const struct ovsdb_error *error) > return json; > } > > +/* Returns 'error' converted to a string suitable for use as an error > message. > + * The caller must free the returned string (with free()). */ > char * > ovsdb_error_to_string(const struct ovsdb_error *error) > { > @@ -240,6 +240,24 @@ ovsdb_error_to_string(const struct ovsdb_error *error) > return ds_steal_cstr(&ds); > } > > +/* Returns 'error' converted to a string suitable for use as an error > message. > + * The caller must free the returned string (with free()). > + * > + * If 'error' is NULL, returns NULL. > + * > + * Also, frees 'error'. */ > +char * > +ovsdb_error_to_string_free(struct ovsdb_error *error) > +{ > + if (error) { > + char *s = ovsdb_error_to_string(error); > + ovsdb_error_destroy(error); > + return s; > + } else { > + return NULL; > + } > +} > + > const char * > ovsdb_error_get_tag(const struct ovsdb_error *error) > { > @@ -254,9 +272,8 @@ ovsdb_error_assert(struct ovsdb_error *error) > { > if (error) { > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > - char *s = ovsdb_error_to_string(error); > + char *s = ovsdb_error_to_string_free(error); > VLOG_ERR_RL(&rl, "unexpected ovsdb error: %s", s); > free(s); > - ovsdb_error_destroy(error); > } > } > diff --git a/lib/ovsdb-error.h b/lib/ovsdb-error.h > index da91b74999d1..ff9b889a8687 100644 > --- a/lib/ovsdb-error.h > +++ b/lib/ovsdb-error.h > @@ -1,4 +1,4 @@ > -/* Copyright (c) 2009, 2010, 2011 Nicira, Inc. > +/* Copyright (c) 2009, 2010, 2011, 2016, 2017 Nicira, Inc. > * > * Licensed under the Apache License, Version 2.0 (the "License"); > * you may not use this file except in compliance with the License. > @@ -64,6 +64,7 @@ struct ovsdb_error *ovsdb_error_clone(const struct > ovsdb_error *) > OVS_WARN_UNUSED_RESULT; > > char *ovsdb_error_to_string(const struct ovsdb_error *); > +char *ovsdb_error_to_string_free(struct ovsdb_error *); > struct json *ovsdb_error_to_json(const struct ovsdb_error *); > > const char *ovsdb_error_get_tag(const struct ovsdb_error *); > diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c > index 2a3405b6f93a..96e5c1f58bbf 100644 > --- a/lib/ovsdb-idl.c > +++ b/lib/ovsdb-idl.c > @@ -1455,10 +1455,9 @@ ovsdb_idl_send_schema_request(struct ovsdb_idl > *idl) > static void > log_error(struct ovsdb_error *error) > { > - char *s = ovsdb_error_to_string(error); > + char *s = ovsdb_error_to_string_free(error); > VLOG_WARN("error parsing database schema: %s", s); > free(s); > - ovsdb_error_destroy(error); > } > > /* Frees 'schema', which is in the format returned by parse_schema(). */ > @@ -1976,12 +1975,11 @@ ovsdb_idl_row_change__(struct ovsdb_idl_row *row, > const struct json *row_json, > > ovsdb_datum_destroy(&datum, &column->type); > } else { > - char *s = ovsdb_error_to_string(error); > + char *s = ovsdb_error_to_string_free(error); > VLOG_WARN_RL(&syntax_rl, "error parsing column %s in row > "UUID_FMT > " in table %s: %s", column_name, > UUID_ARGS(&row->uuid), table->class_->name, s); > free(s); > - ovsdb_error_destroy(error); > } > } > return changed; > @@ -4186,11 +4184,10 @@ ovsdb_idl_txn_process_insert_reply(struct > ovsdb_idl_txn_insert *insert, > > error = ovsdb_atom_from_json(&uuid, &uuid_type, json_uuid, NULL); > if (error) { > - char *s = ovsdb_error_to_string(error); > + char *s = ovsdb_error_to_string_free(error); > VLOG_WARN_RL(&syntax_rl, "\"insert\" reply \"uuid\" is not a JSON > " > "UUID: %s", s); > free(s); > - ovsdb_error_destroy(error); > return false; > } > > diff --git a/ovsdb/file.c b/ovsdb/file.c > index 6a406da2a838..2f07bba3d30c 100644 > --- a/ovsdb/file.c > +++ b/ovsdb/file.c > @@ -237,11 +237,9 @@ ovsdb_file_open__(const char *file_name, > /* Log error but otherwise ignore it. Probably the database just > got > * truncated due to power failure etc. and we should use its > current > * contents. */ > - char *msg = ovsdb_error_to_string(error); > + char *msg = ovsdb_error_to_string_free(error); > VLOG_ERR("%s", msg); > free(msg); > - > - ovsdb_error_destroy(error); > } > > if (!read_only) { > @@ -608,8 +606,7 @@ ovsdb_file_commit(struct ovsdb_replica *replica, > { > error = ovsdb_file_compact(file); > if (error) { > - char *s = ovsdb_error_to_string(error); > - ovsdb_error_destroy(error); > + char *s = ovsdb_error_to_string_free(error); > VLOG_WARN("%s: compacting database failed (%s), retrying in " > "%d seconds", > file->file_name, s, COMPACT_RETRY_MSEC / 1000); > diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c > index 1e0c20486e38..4754a517d316 100644 > --- a/ovsdb/ovsdb-server.c > +++ b/ovsdb/ovsdb-server.c > @@ -916,10 +916,9 @@ update_remote_status(const struct > ovsdb_jsonrpc_server *jsonrpc, > db = node->data; > error = ovsdb_txn_commit(db->txn, false); > if (error) { > - char *msg = ovsdb_error_to_string(error); > + char *msg = ovsdb_error_to_string_free(error); > VLOG_ERR_RL(&rl, "Failed to update remote status: %s", msg); > free(msg); > - ovsdb_error_destroy(error); > } > } > } > @@ -1157,10 +1156,9 @@ ovsdb_server_compact(struct unixctl_conn *conn, int > argc, > > error = ovsdb_file_compact(db->file); > if (error) { > - char *s = ovsdb_error_to_string(error); > + char *s = ovsdb_error_to_string_free(error); > ds_put_format(&reply, "%s\n", s); > free(s); > - ovsdb_error_destroy(error); > } > > n++; > diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c > index 5037258cd4c8..cf9ad0c1938d 100644 > --- a/tests/test-ovsdb.c > +++ b/tests/test-ovsdb.c > @@ -269,8 +269,7 @@ print_and_free_json(struct json *json) > static void > print_and_free_ovsdb_error(struct ovsdb_error *error) > { > - char *string = ovsdb_error_to_string(error); > - ovsdb_error_destroy(error); > + char *string = ovsdb_error_to_string_free(error); > puts(string); > free(string); > } > @@ -279,8 +278,7 @@ static void > check_ovsdb_error(struct ovsdb_error *error) > { > if (error) { > - char *s = ovsdb_error_to_string(error); > - ovsdb_error_destroy(error); > + char *s = ovsdb_error_to_string_free(error); > ovs_fatal(0, "%s", s); > } > } > @@ -344,10 +342,9 @@ do_log_io(struct ovs_cmdl_context *ctx) > ovs_fatal(0, "unknown log-io command \"%s\"", command); > } > if (error) { > - char *s = ovsdb_error_to_string(error); > + char *s = ovsdb_error_to_string_free(error); > printf("%s: %s failed: %s\n", name, command, s); > free(s); > - ovsdb_error_destroy(error); > } else { > printf("%s: %s successful\n", name, command); > } > @@ -450,8 +447,7 @@ do_diff_data(struct ovs_cmdl_context *ctx) > /* Apply diff to 'old' to create'reincarnation'. */ > error = ovsdb_datum_apply_diff(&reincarnation, &old, &diff, > &type); > if (error) { > - char *string = ovsdb_error_to_string(error); > - ovsdb_error_destroy(error); > + char *string = ovsdb_error_to_string_free(error); > ovs_fatal(0, "%s", string); > } > > @@ -873,10 +869,9 @@ do_parse_conditions(struct ovs_cmdl_context *ctx) > if (!error) { > print_and_free_json(ovsdb_condition_to_json(&cnd)); > } else { > - char *s = ovsdb_error_to_string(error); > + char *s = ovsdb_error_to_string_free(error); > ovs_error(0, "%s", s); > free(s); > - ovsdb_error_destroy(error); > exit_code = 1; > } > json_destroy(json); > @@ -1044,10 +1039,9 @@ do_parse_mutations(struct ovs_cmdl_context *ctx) > if (!error) { > print_and_free_json(ovsdb_mutation_set_to_json(&set)); > } else { > - char *s = ovsdb_error_to_string(error); > + char *s = ovsdb_error_to_string_free(error); > ovs_error(0, "%s", s); > free(s); > - ovsdb_error_destroy(error); > exit_code = 1; > } > json_destroy(json); > -- > 2.10.2 > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev