On Sat, Apr 15, 2023 at 11:21:51PM +0800, James Raphael Tiovalen wrote:
> This commit adds more verbose logging in null-pointer check blocks by
> using `ovsdb_syntax_error` and `VLOG_ERR`. A few additional null pointer
> checks are also performed to accompany the additional logging. If a null
> pointer is encountered in these blocks, the control flow will now be
> redirected to alternative paths which will output the appropriate error
> messages.
>
> Signed-off-by: James Raphael Tiovalen <[email protected]>
> ---
> ovsdb/condition.c | 10 +++++++++-
> ovsdb/monitor.c | 3 ++-
> ovsdb/ovsdb-client.c | 10 ++++++++--
> ovsdb/ovsdb-util.c | 9 +++++++++
> ovsdb/table.c | 2 +-
> 5 files changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/ovsdb/condition.c b/ovsdb/condition.c
> index d0016fa7f..ada8f50dc 100644
> --- a/ovsdb/condition.c
> +++ b/ovsdb/condition.c
> @@ -47,7 +47,15 @@ ovsdb_clause_from_json(const struct ovsdb_table_schema *ts,
>
> /* Column and arg fields are not being used with boolean functions.
> * Use dummy values */
> - clause->column = ovsdb_table_schema_get_column(ts, "_uuid");
> + const struct ovsdb_column *uuid_column =
> + ovsdb_table_schema_get_column(ts, "_uuid");
> + if (uuid_column) {
> + clause->column = uuid_column;
> + } else {
> + return ovsdb_syntax_error(json,
> + "unknown column",
> + "No column _uuid in table schema.");
> + }
Hi James,
FWIIW, I would lean towards writing this as:
if (!uuid_column) {
return ovsdb_syntax_error(json,
"unknown column",
"No column _uuid in table schema.");
}
clause->column = uuid_column;
> clause->index = clause->column->index;
> ovsdb_datum_init_default(&clause->arg, &clause->column->type);
> return NULL;
> diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
> index 191befcae..4a0d6512f 100644
> --- a/ovsdb/monitor.c
> +++ b/ovsdb/monitor.c
> @@ -781,7 +781,8 @@ ovsdb_monitor_table_condition_update(
> const struct json *cond_json)
> {
> if (!condition) {
> - return NULL;
> + return ovsdb_syntax_error(cond_json, NULL,
> + "Parse error, condition empty.");
> }
>
> struct ovsdb_monitor_table_condition *mtc =
> diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
> index bae2c5f04..41dfeb3b6 100644
> --- a/ovsdb/ovsdb-client.c
> +++ b/ovsdb/ovsdb-client.c
> @@ -1232,8 +1232,14 @@ parse_monitor_columns(char *arg, const char *server,
> const char *database,
> }
> free(nodes);
>
> - add_column(server, ovsdb_table_schema_get_column(table, "_version"),
> - columns, columns_json);
> + const struct ovsdb_column *version_column =
> + ovsdb_table_schema_get_column(table, "_version");
> +
> + if (version_column) {
> + add_column(server, version_column, columns, columns_json);
> + } else {
> + VLOG_ERR("Table does not contain _version column.");
> + }
> }
>
> if (!initial || !insert || !delete || !modify) {
> diff --git a/ovsdb/ovsdb-util.c b/ovsdb/ovsdb-util.c
> index 303191dc8..148230da8 100644
> --- a/ovsdb/ovsdb-util.c
> +++ b/ovsdb/ovsdb-util.c
> @@ -291,6 +291,15 @@ ovsdb_util_write_string_string_column(struct ovsdb_row
> *row,
> size_t i;
>
> column = ovsdb_table_schema_get_column(row->table->schema, column_name);
> + if (!column) {
> + VLOG_ERR("No %s column present in the %s table",
> + column_name, row->table->schema->name);
The unwind logic below seems to already exist in this function.
So rather than duplicating it, could it be moved to a goto label?
> + for (i = 0; i < n; i++) {
> + free(keys[i]);
> + free(values[i]);
> + }
> + return;
> + }
> datum = ovsdb_util_get_datum(row, column_name, OVSDB_TYPE_STRING,
> OVSDB_TYPE_STRING, UINT_MAX);
> if (!datum) {
> diff --git a/ovsdb/table.c b/ovsdb/table.c
> index 66071ce2f..990d49ea4 100644
> --- a/ovsdb/table.c
> +++ b/ovsdb/table.c
> @@ -158,7 +158,7 @@ ovsdb_table_schema_from_json(const struct json *json,
> const char *name,
> n_max_rows = UINT_MAX;
> }
>
> - if (shash_is_empty(json_object(columns))) {
> + if (!columns || shash_is_empty(json_object(columns))) {
> return ovsdb_syntax_error(json, NULL,
> "table must have at least one column");
> }
> --
> 2.40.0
>
> _______________________________________________
> 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
