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

Reply via email to