Re: [ovs-dev] [PATCH 1/3] ovsdb-error: New function ovsdb_error_to_string_free().

2017-12-13 Thread Yifeng Sun 
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, >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 

Reviewed-by: Yifeng Sun 

On Wed, Dec 13, 2017 at 10:04 AM, Ben Pfaff  wrote:

> This allows slight code simplifications across the tree.
>
> Signed-off-by: Ben Pfaff 
> ---
>  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(, " (%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(, " (generated from: %s)", s);
>  free(s);
> -
> -ovsdb_error_destroy(inner_error);
>  }
>
>  error = ovsdb_error("internal error", "%s", ds_cstr());
> @@ -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();
>  }
>
> +/* 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(, "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

[ovs-dev] [PATCH 1/3] ovsdb-error: New function ovsdb_error_to_string_free().

2017-12-13 Thread Ben Pfaff 
This allows slight code simplifications across the tree.

Signed-off-by: Ben Pfaff 
---
 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(, " (%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(, " (generated from: %s)", s);
 free(s);
-
-ovsdb_error_destroy(inner_error);
 }
 
 error = ovsdb_error("internal error", "%s", ds_cstr());
@@ -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();
 }
 
+/* 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(, "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(, >type);
 } else {
-char *s = ovsdb_error_to_string(error);
+char *s = ovsdb_error_to_string_free(error);
 VLOG_WARN_RL(_rl, "error parsing