| Message ID | 20250606112253.3314013-2-i.maximets@ovn.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | json: Store short strings and arrays in-place. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
On 6/6/25 1:22 PM, Ilya Maximets wrote: > We'll be changing the way strings are stored, so the direct access > will not be safe anymore. Change all the users to use the proper > API as they should have been doing anyway. > > The only code outside of json implementation for which direct access > is preserved is substitute_uuids() in test-ovsdb.c. It's an unusual > string manipulation that is only needed for the testing, so doesn't > seem worthy adding a new API function. We could introduce something > like json_string_replace() if this use case will appear somewhere > else in the future. > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- GitHub decided to cancel the job. Recheck-request: github-robot
On Fri, Jun 6, 2025 at 7:23 AM Ilya Maximets <i.maximets@ovn.org> wrote: > > We'll be changing the way strings are stored, so the direct access > will not be safe anymore. Change all the users to use the proper > API as they should have been doing anyway. > > The only code outside of json implementation for which direct access > is preserved is substitute_uuids() in test-ovsdb.c. It's an unusual > string manipulation that is only needed for the testing, so doesn't > seem worthy adding a new API function. We could introduce something > like json_string_replace() if this use case will appear somewhere > else in the future. > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- > lib/db-ctl-base.c | 8 +++++--- > lib/json.c | 12 ++++++------ > lib/jsonrpc.c | 4 ++-- > lib/ovsdb-cs.c | 2 +- > lib/ovsdb-data.c | 4 ++-- > lib/ovsdb-idl.c | 16 +++++++++------- > lib/ovsdb-parser.c | 2 +- > lib/ovsdb-types.c | 4 ++-- > ovsdb/column.c | 2 +- > ovsdb/execution.c | 2 +- > ovsdb/jsonrpc-server.c | 6 +++--- > ovsdb/log.c | 2 +- > ovsdb/ovsdb-client.c | 4 ++-- > ovsdb/ovsdb-idlc.in | 12 ++++++++++-- > ovsdb/ovsdb-tool.c | 2 +- > ovsdb/ovsdb-util.c | 8 ++++---- > ovsdb/replication.c | 2 +- > python/ovs/_json.c | 2 +- > python/ovs/db/types.py | 2 +- > tests/test-json.c | 6 +++--- > tests/test-jsonrpc.c | 2 +- > tests/test-ovsdb.c | 7 ++++--- > 22 files changed, 62 insertions(+), 49 deletions(-) > > diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c > index 1f157e46c..eee9646de 100644 > --- a/lib/db-ctl-base.c > +++ b/lib/db-ctl-base.c > @@ -247,15 +247,17 @@ record_id_equals(const union ovsdb_atom *name, enum ovsdb_atomic_type type, > const char *record_id) > { > if (type == OVSDB_TYPE_STRING) { > - if (!strcmp(name->s->string, record_id)) { > + const char *name_str = json_string(name->s); > + > + if (!strcmp(name_str, record_id)) { > return true; > } > > struct uuid uuid; > size_t len = strlen(record_id); > if (len >= 4 > - && uuid_from_string(&uuid, name->s->string) > - && !strncmp(name->s->string, record_id, len)) { > + && uuid_from_string(&uuid, name_str) > + && !strncmp(name_str, record_id, len)) { > return true; > } > > diff --git a/lib/json.c b/lib/json.c > index 2649e8e12..20626c445 100644 > --- a/lib/json.c > +++ b/lib/json.c > @@ -493,7 +493,7 @@ json_deep_clone(const struct json *json) > return json_deep_clone_array(&json->array); > > case JSON_STRING: > - return json_string_create(json->string); > + return json_string_create(json_string(json)); > > case JSON_SERIALIZED_OBJECT: > return json_serialized_object_create(json); > @@ -589,7 +589,7 @@ json_hash(const struct json *json, size_t basis) > > case JSON_STRING: > case JSON_SERIALIZED_OBJECT: > - return hash_string(json->string, basis); > + return hash_string(json_string(json), basis); Hello Ilya, I got a SIGABORT here due to json->type != JSON_STRING in json_string(). Stack trace: #3 0x00007ffff76a36d0 in __GI_abort () at abort.c:73 #4 0x0000000000472664 in ovs_abort_valist (err_no=0, format=0x6298f8 "%s: assertion %s failed in %s()", args=0x7fffffffd428) at lib/util.c:450 #5 0x0000000000478f35 in vlog_abort_valist (module_=0x6f6600 <this_module>, message=0x6298f8 "%s: assertion %s failed in %s()", args=0x7fffffffd428) at lib/vlog.c:1311 #6 0x0000000000478fdb in vlog_abort (module=0x6f6600 <this_module>, message=0x6298f8 "%s: assertion %s failed in %s()") at lib/vlog.c:1325 #7 0x0000000000471e57 in ovs_assert_failure (where=0x622c14 "lib/json.c:359", function=0x623540 <__func__.8> "json_string", condition=0x622bfa "json->type == JSON_STRING") at lib/util.c:90 #8 0x0000000000446584 in json_string (json=0x7cc890) at lib/json.c:359 #9 0x0000000000448f58 in json_serialize (json=0x7cc890, s=0x7fffffffd670) at lib/json.c:1663 #10 0x0000000000449066 in json_serialize_object_member (i=1, node=0x7cc450, s=0x7fffffffd670) at lib/json.c:1697 #11 0x0000000000449183 in json_serialize_object (object=0x7cc330, s=0x7fffffffd670) at lib/json.c:1730 #12 0x0000000000448ec1 in json_serialize (json=0x7cc360, s=0x7fffffffd670) at lib/json.c:1643 #13 0x0000000000448dc0 in json_to_ds (json=0x7cc360, flags=0, ds=0x7fffffffd6a0) at lib/json.c:1621 #14 0x0000000000449cef in jsonrpc_send (rpc=0x79f7d0, msg=0x7cc1e0) at lib/jsonrpc.c:273 #15 0x000000000044bb5c in jsonrpc_session_send (s=0x7ce8f0, msg=0x7cc1e0) at lib/jsonrpc.c:1170 #16 0x000000000040daa0 in ovsdb_jsonrpc_session_send (s=0x7a0d40, msg=0x7cc1e0) at ovsdb/jsonrpc-server.c:1213 #17 0x000000000040d99b in ovsdb_jsonrpc_session_got_request (s=0x7a0d40, request=0x7a0b70) at ovsdb/jsonrpc-server.c:1179 #18 0x000000000040c4fe in ovsdb_jsonrpc_session_run (s=0x7a0d40) at ovsdb/jsonrpc-server.c:677 #19 0x000000000040c5fa in ovsdb_jsonrpc_session_run_all (remote=0x759cd0) at ovsdb/jsonrpc-server.c:700 #20 0x000000000040c08b in ovsdb_jsonrpc_server_run (svr=0x72e480) at ovsdb/jsonrpc-server.c:522 #21 0x000000000040200f in main_loop (config=0x7fffffffd9c0, jsonrpc=0x72e480, all_dbs=0x7fffffffda90, unixctl=0x79d230, remotes=0x7fffffffda20, run_process=0x0, exiting=0x7fffffffdabf) at ovsdb/ovsdb-server.c:331 #22 0x00000000004038d8 in main (argc=5, argv=0x7fffffffdc48) at ovsdb/ovsdb-server.c:891 Cheers, M > > case JSON_NULL: > case JSON_FALSE: > @@ -665,7 +665,7 @@ json_equal(const struct json *a, const struct json *b) > > case JSON_STRING: > case JSON_SERIALIZED_OBJECT: > - return !strcmp(a->string, b->string); > + return !strcmp(json_string(a), json_string(b)); > > case JSON_NULL: > case JSON_FALSE: > @@ -1064,7 +1064,7 @@ struct json * > json_from_serialized_object(const struct json *json) > { > ovs_assert(json->type == JSON_SERIALIZED_OBJECT); > - return json_from_string(json->string); > + return json_from_string(json_string(json)); > } > > /* Reads the file named 'file_name', parses its contents as a JSON object or > @@ -1656,11 +1656,11 @@ json_serialize(const struct json *json, struct json_serializer *s) > break; > > case JSON_STRING: > - json_serialize_string(json->string, ds); > + json_serialize_string(json_string(json), ds); > break; > > case JSON_SERIALIZED_OBJECT: > - ds_put_cstr(ds, json->string); > + ds_put_cstr(ds, json_string(json)); > break; > > case JSON_N_TYPES: > diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c > index 90723a42b..f01a2e56f 100644 > --- a/lib/jsonrpc.c > +++ b/lib/jsonrpc.c > @@ -737,7 +737,7 @@ jsonrpc_msg_from_json(struct json *json, struct jsonrpc_msg **msgp) > } > > msg = xzalloc(sizeof *msg); > - msg->method = method ? xstrdup(method->string) : NULL; > + msg->method = method ? xstrdup(json_string(method)) : NULL; > msg->params = null_from_json_null(shash_find_and_delete(object, "params")); > msg->result = null_from_json_null(shash_find_and_delete(object, "result")); > msg->error = null_from_json_null(shash_find_and_delete(object, "error")); > @@ -1204,7 +1204,7 @@ jsonrpc_session_recv(struct jsonrpc_session *s) > jsonrpc_session_send(s, reply); > } else if (msg->type == JSONRPC_REPLY > && msg->id && msg->id->type == JSON_STRING > - && !strcmp(msg->id->string, "echo")) { > + && !strcmp(json_string(msg->id), "echo")) { > /* It's a reply to our echo request. Suppress it. */ > } else { > return msg; > diff --git a/lib/ovsdb-cs.c b/lib/ovsdb-cs.c > index 87c6211bf..20386101f 100644 > --- a/lib/ovsdb-cs.c > +++ b/lib/ovsdb-cs.c > @@ -1878,7 +1878,7 @@ server_column_get_string(const struct server_row *row, > { > ovs_assert(server_columns[index].type.key.type == OVSDB_TYPE_STRING); > const struct ovsdb_datum *d = &row->data[index]; > - return d->n == 1 ? d->keys[0].s->string : default_value; > + return d->n == 1 ? json_string(d->keys[0].s) : default_value; > } > > static bool > diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c > index e4149401f..d38eee5d0 100644 > --- a/lib/ovsdb-data.c > +++ b/lib/ovsdb-data.c > @@ -264,7 +264,7 @@ unwrap_json(const struct json *json, const char *name, > if (json->type != JSON_ARRAY > || json->array.n != 2 > || json->array.elems[0]->type != JSON_STRING > - || (name && strcmp(json->array.elems[0]->string, name)) > + || (name && strcmp(json_string(json->array.elems[0]), name)) > || json->array.elems[1]->type != value_type) > { > *value = NULL; > @@ -1278,7 +1278,7 @@ ovsdb_datum_from_json__(struct ovsdb_datum *datum, > || (json->type == JSON_ARRAY > && json->array.n > 0 > && json->array.elems[0]->type == JSON_STRING > - && !strcmp(json->array.elems[0]->string, "set"))) { > + && !strcmp(json_string(json->array.elems[0]), "set"))) { > bool is_map = ovsdb_type_is_map(type); > const char *class = is_map ? "map" : "set"; > const struct json *inner; > diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c > index 226a4b475..cef4a337b 100644 > --- a/lib/ovsdb-idl.c > +++ b/lib/ovsdb-idl.c > @@ -2865,8 +2865,8 @@ substitute_uuids(struct json *json, const struct ovsdb_idl_txn *txn) > if (json->array.n == 2 > && json->array.elems[0]->type == JSON_STRING > && json->array.elems[1]->type == JSON_STRING > - && !strcmp(json->array.elems[0]->string, "uuid") > - && uuid_from_string(&uuid, json->array.elems[1]->string)) { > + && !strcmp(json_string(json->array.elems[0]), "uuid") > + && uuid_from_string(&uuid, json_string(json->array.elems[1]))) { > const struct ovsdb_idl_row *row; > > row = ovsdb_idl_txn_get_row(txn, &uuid); > @@ -4038,18 +4038,20 @@ ovsdb_idl_txn_process_reply(struct ovsdb_idl *idl, > error = shash_find_data(json_object(op), "error"); > if (error) { > if (error->type == JSON_STRING) { > - if (!strcmp(error->string, "timed out")) { > + const char *error_string = json_string(error); > + > + if (!strcmp(error_string, "timed out")) { > soft_errors++; > - } else if (!strcmp(error->string, > + } else if (!strcmp(error_string, > "unknown database")) { > ovsdb_cs_flag_inconsistency(idl->cs); > soft_errors++; > - } else if (!strcmp(error->string, "not owner")) { > + } else if (!strcmp(error_string, "not owner")) { > lock_errors++; > - } else if (!strcmp(error->string, "not allowed")) { > + } else if (!strcmp(error_string, "not allowed")) { > hard_errors++; > ovsdb_idl_txn_set_error_json(txn, op); > - } else if (strcmp(error->string, "aborted")) { > + } else if (strcmp(error_string, "aborted")) { > hard_errors++; > ovsdb_idl_txn_set_error_json(txn, op); > VLOG_WARN_RL(&other_rl, > diff --git a/lib/ovsdb-parser.c b/lib/ovsdb-parser.c > index 38119c195..07a670781 100644 > --- a/lib/ovsdb-parser.c > +++ b/lib/ovsdb-parser.c > @@ -83,7 +83,7 @@ ovsdb_parser_member(struct ovsdb_parser *parser, const char *name, > if (((int) value->type >= 0 && value->type < JSON_N_TYPES > && types & (1u << value->type)) > || (types & OP_ID && value->type == JSON_STRING > - && ovsdb_parser_is_id(value->string))) > + && ovsdb_parser_is_id(json_string(value)))) > { > sset_add(&parser->used, name); > return value; > diff --git a/lib/ovsdb-types.c b/lib/ovsdb-types.c > index 69f132597..fce540f4d 100644 > --- a/lib/ovsdb-types.c > +++ b/lib/ovsdb-types.c > @@ -476,7 +476,7 @@ ovsdb_base_type_from_json(struct ovsdb_base_type *base, > if (refTable) { > const struct json *refType; > > - base->uuid.refTableName = xstrdup(refTable->string); > + base->uuid.refTableName = xstrdup(json_string(refTable)); > > /* We can't set base->uuid.refTable here because we don't have > * enough context (we might not even be running in ovsdb-server). > @@ -718,7 +718,7 @@ ovsdb_type_from_json(struct ovsdb_type *type, const struct json *json) > } > > if (max && max->type == JSON_STRING > - && !strcmp(max->string, "unlimited")) { > + && !strcmp(json_string(max), "unlimited")) { > type->n_max = UINT_MAX; > } else { > error = n_from_json(max, &type->n_max); > diff --git a/ovsdb/column.c b/ovsdb/column.c > index 9dfc8714f..37cda730c 100644 > --- a/ovsdb/column.c > +++ b/ovsdb/column.c > @@ -173,7 +173,7 @@ ovsdb_column_set_from_json(const struct json *json, > goto error; > } > > - s = json->array.elems[i]->string; > + s = json_string(json->array.elems[i]); > column = shash_find_data(&schema->columns, s); > if (!column) { > error = ovsdb_syntax_error(json, NULL, "%s is not a valid " > diff --git a/ovsdb/execution.c b/ovsdb/execution.c > index f4cc9e802..756129da4 100644 > --- a/ovsdb/execution.c > +++ b/ovsdb/execution.c > @@ -128,7 +128,7 @@ ovsdb_execute_compose(struct ovsdb *db, const struct ovsdb_session *session, > if (params->type != JSON_ARRAY > || !params->array.n > || params->array.elems[0]->type != JSON_STRING > - || strcmp(params->array.elems[0]->string, db->schema->name)) { > + || strcmp(json_string(params->array.elems[0]), db->schema->name)) { > if (params->type != JSON_ARRAY) { > error = ovsdb_syntax_error(params, NULL, "array expected"); > } else { > diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c > index ba5a4211b..50fb2c7f1 100644 > --- a/ovsdb/jsonrpc-server.c > +++ b/ovsdb/jsonrpc-server.c > @@ -900,7 +900,7 @@ ovsdb_jsonrpc_lookup_db(const struct ovsdb_jsonrpc_session *s, > goto error; > } > > - db_name = params->elems[0]->string; > + db_name = json_string(params->elems[0]); > db = shash_find_data(&s->up.server->dbs, db_name); > if (!db) { > error = ovsdb_syntax_error( > @@ -1447,7 +1447,7 @@ ovsdb_jsonrpc_parse_monitor_request( > "array of column names expected"); > } > > - s = columns->array.elems[i]->string; > + s = json_string(columns->array.elems[i]); > column = shash_find_data(&table->schema->columns, s); > if (!column) { > return ovsdb_syntax_error(columns, NULL, "%s is not a valid " > @@ -1590,7 +1590,7 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session *s, struct ovsdb *db, > goto error; > } > struct uuid txn_uuid; > - if (!uuid_from_string(&txn_uuid, last_id->string)) { > + if (!uuid_from_string(&txn_uuid, json_string(last_id))) { > error = ovsdb_syntax_error(last_id, NULL, > "last-txn-id must be UUID format."); > goto error; > diff --git a/ovsdb/log.c b/ovsdb/log.c > index fff7c6ba1..0c3577119 100644 > --- a/ovsdb/log.c > +++ b/ovsdb/log.c > @@ -495,7 +495,7 @@ ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp) > "offset %lld are not valid JSON (%s)", > file->display_name, data_length, > (long long int) data_offset, > - json->string); > + json_string(json)); > goto error; > } > if (json->type != JSON_OBJECT) { > diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c > index 5258eccaa..f85057b6c 100644 > --- a/ovsdb/ovsdb-client.c > +++ b/ovsdb/ovsdb-client.c > @@ -501,7 +501,7 @@ parse_json(const char *s) > { > struct json *json = json_from_string(s); > if (json->type == JSON_STRING) { > - ovs_fatal(0, "\"%s\": %s", s, json->string); > + ovs_fatal(0, "\"%s\": %s", s, json_string(json)); > } > return json; > } > @@ -596,7 +596,7 @@ fetch_dbs(struct jsonrpc *rpc, struct svec *dbs) > if (name->type != JSON_STRING) { > ovs_fatal(0, "list_dbs response %"PRIuSIZE" is not string", i); > } > - svec_add(dbs, name->string); > + svec_add(dbs, json_string(name)); > } > jsonrpc_msg_destroy(reply); > svec_sort(dbs); > diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in > index 9a54f06a1..5c3e76c77 100755 > --- a/ovsdb/ovsdb-idlc.in > +++ b/ovsdb/ovsdb-idlc.in > @@ -601,13 +601,17 @@ static void > elif (type.n_min == 1 and type.n_max == 1) or type.is_optional_pointer(): > print("") > print(" if (datum->n >= 1) {") > - if not type.key.ref_table: > + if type.key.type == ovs.db.types.StringType: > + print(" %s = CONST_CAST(char *, json_string(datum->keys[0].s));" % keyVar) > + elif not type.key.ref_table: > print(" %s = datum->keys[0].%s;" % (keyVar, type.key.type.to_rvalue_string())) > else: > print(" %s = %s%s_cast(ovsdb_idl_get_row_arc(row_, &%stable_%s, &datum->keys[0].uuid));" % (keyVar, prefix, type.key.ref_table.name.lower(), prefix, type.key.ref_table.name.lower())) > > if valueVar: > - if not type.value.ref_table: > + if type.value.type == ovs.db.types.StringType: > + print(" %s = CONST_CAST(char *, json_string(datum->values[0].s));" % valueVar) > + elif not type.value.ref_table: > print(" %s = datum->values[0].%s;" % (valueVar, type.value.type.to_rvalue_string())) > else: > print(" %s = %s%s_cast(ovsdb_idl_get_row_arc(row_, &%stable_%s, &datum->values[0].uuid));" % (valueVar, prefix, type.value.ref_table.name.lower(), prefix, type.value.ref_table.name.lower())) > @@ -635,6 +639,8 @@ static void > }\ > """ % (prefix, type.key.ref_table.name.lower(), prefix, type.key.ref_table.name.lower(), prefix, type.key.ref_table.name.lower())) > keySrc = "keyRow" > + elif type.key.type == ovs.db.types.StringType: > + keySrc = "CONST_CAST(char *, json_string(datum->keys[i].s))" > else: > keySrc = "datum->keys[i].%s" % type.key.type.to_rvalue_string() > if type.value and type.value.ref_table: > @@ -645,6 +651,8 @@ static void > }\ > """ % (prefix, type.value.ref_table.name.lower(), prefix, type.value.ref_table.name.lower(), prefix, type.value.ref_table.name.lower())) > valueSrc = "valueRow" > + elif valueVar and type.value.type == ovs.db.types.StringType: > + valueSrc = "CONST_CAST(char *, json_string(datum->values[i].s))" > elif valueVar: > valueSrc = "datum->values[i].%s" % type.value.type.to_rvalue_string() > print(" if (!row->n_%s) {" % (columnName)) > diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c > index 354382f11..9051020b3 100644 > --- a/ovsdb/ovsdb-tool.c > +++ b/ovsdb/ovsdb-tool.c > @@ -229,7 +229,7 @@ parse_json(const char *s) > { > struct json *json = json_from_string(s); > if (json->type == JSON_STRING) { > - ovs_fatal(0, "\"%s\": %s", s, json->string); > + ovs_fatal(0, "\"%s\": %s", s, json_string(json)); > } > return json; > } > diff --git a/ovsdb/ovsdb-util.c b/ovsdb/ovsdb-util.c > index ec4537890..c15e6bb64 100644 > --- a/ovsdb/ovsdb-util.c > +++ b/ovsdb/ovsdb-util.c > @@ -111,13 +111,13 @@ ovsdb_util_read_map_string_column(const struct ovsdb_row *row, > > for (i = 0; i < datum->n; i++) { > atom_key = &datum->keys[i]; > - if (!strcmp(atom_key->s->string, key)) { > + if (!strcmp(json_string(atom_key->s), key)) { > atom_value = &datum->values[i]; > break; > } > } > > - return atom_value ? atom_value->s->string : NULL; > + return atom_value ? json_string(atom_value->s) : NULL; > } > > /* Read string-uuid key-values from a map. Returns the row associated with > @@ -143,7 +143,7 @@ ovsdb_util_read_map_string_uuid_column(const struct ovsdb_row *row, > const struct ovsdb_datum *datum = &row->fields[column->index]; > for (size_t i = 0; i < datum->n; i++) { > union ovsdb_atom *atom_key = &datum->keys[i]; > - if (!strcmp(atom_key->s->string, key)) { > + if (!strcmp(json_string(atom_key->s), key)) { > const union ovsdb_atom *atom_value = &datum->values[i]; > return ovsdb_table_get_row(ref_table, &atom_value->uuid); > } > @@ -181,7 +181,7 @@ ovsdb_util_read_string_column(const struct ovsdb_row *row, > const union ovsdb_atom *atom; > > atom = ovsdb_util_read_column(row, column_name, OVSDB_TYPE_STRING); > - *stringp = atom ? atom->s->string : NULL; > + *stringp = atom ? json_string(atom->s) : NULL; > return atom != NULL; > } > > diff --git a/ovsdb/replication.c b/ovsdb/replication.c > index 56720cb10..67eab9c42 100644 > --- a/ovsdb/replication.c > +++ b/ovsdb/replication.c > @@ -220,7 +220,7 @@ replication_run_db(struct replication_db *rdb) > if (msg->params->type == JSON_ARRAY > && msg->params->array.n == 2 > && msg->params->array.elems[0]->type == JSON_STRING) { > - char *db_name = msg->params->array.elems[0]->string; > + const char *db_name = json_string(msg->params->array.elems[0]); > > if (!strcmp(db_name, rdb->db->name)) { > struct ovsdb_error *error; > diff --git a/python/ovs/_json.c b/python/ovs/_json.c > index afa97896d..5f4388f80 100644 > --- a/python/ovs/_json.c > +++ b/python/ovs/_json.c > @@ -135,7 +135,7 @@ json_to_python(struct json *json) > return PyLong_FromLong((long) json->integer); > > case JSON_STRING: > - return PyUnicode_FromString(json->string); > + return PyUnicode_FromString(json_string(json)); > default: > return NULL; > } > diff --git a/python/ovs/db/types.py b/python/ovs/db/types.py > index 3318a3b6f..af6aa878c 100644 > --- a/python/ovs/db/types.py > +++ b/python/ovs/db/types.py > @@ -50,7 +50,7 @@ class AtomicType(object): > > def to_rvalue_string(self): > if self == StringType: > - return 's->' + self.name > + raise error.Error("can't convert string type to rvalue") > return self.name > > def to_lvalue_string(self): > diff --git a/tests/test-json.c b/tests/test-json.c > index 6cf5eb75d..2ba92d119 100644 > --- a/tests/test-json.c > +++ b/tests/test-json.c > @@ -101,8 +101,8 @@ test_json_equal(const struct json *a, const struct json *b, > > case JSON_STRING: > case JSON_SERIALIZED_OBJECT: > - ovs_assert(a->string != b->string); > - ovs_assert(!strcmp(a->string, b->string)); > + ovs_assert(json_string(a) != json_string(b)); > + ovs_assert(!strcmp(json_string(a), json_string(b))); > return; > > case JSON_NULL: > @@ -154,7 +154,7 @@ print_test_and_free_json(struct json *json) > { > bool ok; > if (json->type == JSON_STRING) { > - printf("error: %s\n", json->string); > + printf("error: %s\n", json_string(json)); > ok = false; > } else { > char *s = json_to_string(json, JSSF_SORT | (pretty ? JSSF_PRETTY : 0)); > diff --git a/tests/test-jsonrpc.c b/tests/test-jsonrpc.c > index 04e941b14..02504c289 100644 > --- a/tests/test-jsonrpc.c > +++ b/tests/test-jsonrpc.c > @@ -124,7 +124,7 @@ parse_json(const char *s) > { > struct json *json = json_from_string(s); > if (json->type == JSON_STRING) { > - ovs_fatal(0, "\"%s\": %s", s, json->string); > + ovs_fatal(0, "\"%s\": %s", s, json_string(json)); > } > return json; > } > diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c > index 52438e677..0511e4170 100644 > --- a/tests/test-ovsdb.c > +++ b/tests/test-ovsdb.c > @@ -265,7 +265,7 @@ parse_json(const char *s) > { > struct json *json = json_from_string(s); > if (json->type == JSON_STRING) { > - ovs_fatal(0, "\"%s\": %s", s, json->string); > + ovs_fatal(0, "\"%s\": %s", s, json_string(json)); > } > return json; > } > @@ -937,7 +937,7 @@ do_compare_rows(struct ovs_cmdl_context *ctx) > ovs_fatal(0, "\"%s\" does not have expected form " > "[\"name\", {data}]", ctx->argv[i]); > } > - names[i] = xstrdup(json->array.elems[0]->string); > + names[i] = xstrdup(json_string(json->array.elems[0])); > check_ovsdb_error(ovsdb_row_from_json(rows[i], json->array.elems[1], > NULL, NULL, false)); > json_destroy(json); > @@ -2385,7 +2385,8 @@ parse_uuids(const struct json *json, struct ovsdb_symbol_table *symtab, > { > struct uuid uuid; > > - if (json->type == JSON_STRING && uuid_from_string(&uuid, json->string)) { > + if (json->type == JSON_STRING > + && uuid_from_string(&uuid, json_string(json))) { > char *name = xasprintf("#%"PRIuSIZE"#", *n); > fprintf(stderr, "%s = "UUID_FMT"\n", name, UUID_ARGS(&uuid)); > ovsdb_symbol_table_put(symtab, name, &uuid, false); > -- > 2.49.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 6/6/25 8:13 PM, Mike Pattrick wrote: > On Fri, Jun 6, 2025 at 7:23 AM Ilya Maximets <i.maximets@ovn.org> wrote: >> >> We'll be changing the way strings are stored, so the direct access >> will not be safe anymore. Change all the users to use the proper >> API as they should have been doing anyway. >> >> The only code outside of json implementation for which direct access >> is preserved is substitute_uuids() in test-ovsdb.c. It's an unusual >> string manipulation that is only needed for the testing, so doesn't >> seem worthy adding a new API function. We could introduce something >> like json_string_replace() if this use case will appear somewhere >> else in the future. >> >> Signed-off-by: Ilya Maximets <i.maximets@ovn.org> >> --- >> lib/db-ctl-base.c | 8 +++++--- >> lib/json.c | 12 ++++++------ >> lib/jsonrpc.c | 4 ++-- >> lib/ovsdb-cs.c | 2 +- >> lib/ovsdb-data.c | 4 ++-- >> lib/ovsdb-idl.c | 16 +++++++++------- >> lib/ovsdb-parser.c | 2 +- >> lib/ovsdb-types.c | 4 ++-- >> ovsdb/column.c | 2 +- >> ovsdb/execution.c | 2 +- >> ovsdb/jsonrpc-server.c | 6 +++--- >> ovsdb/log.c | 2 +- >> ovsdb/ovsdb-client.c | 4 ++-- >> ovsdb/ovsdb-idlc.in | 12 ++++++++++-- >> ovsdb/ovsdb-tool.c | 2 +- >> ovsdb/ovsdb-util.c | 8 ++++---- >> ovsdb/replication.c | 2 +- >> python/ovs/_json.c | 2 +- >> python/ovs/db/types.py | 2 +- >> tests/test-json.c | 6 +++--- >> tests/test-jsonrpc.c | 2 +- >> tests/test-ovsdb.c | 7 ++++--- >> 22 files changed, 62 insertions(+), 49 deletions(-) >> >> diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c >> index 1f157e46c..eee9646de 100644 >> --- a/lib/db-ctl-base.c >> +++ b/lib/db-ctl-base.c >> @@ -247,15 +247,17 @@ record_id_equals(const union ovsdb_atom *name, enum ovsdb_atomic_type type, >> const char *record_id) >> { >> if (type == OVSDB_TYPE_STRING) { >> - if (!strcmp(name->s->string, record_id)) { >> + const char *name_str = json_string(name->s); >> + >> + if (!strcmp(name_str, record_id)) { >> return true; >> } >> >> struct uuid uuid; >> size_t len = strlen(record_id); >> if (len >= 4 >> - && uuid_from_string(&uuid, name->s->string) >> - && !strncmp(name->s->string, record_id, len)) { >> + && uuid_from_string(&uuid, name_str) >> + && !strncmp(name_str, record_id, len)) { >> return true; >> } >> >> diff --git a/lib/json.c b/lib/json.c >> index 2649e8e12..20626c445 100644 >> --- a/lib/json.c >> +++ b/lib/json.c >> @@ -493,7 +493,7 @@ json_deep_clone(const struct json *json) >> return json_deep_clone_array(&json->array); >> >> case JSON_STRING: >> - return json_string_create(json->string); >> + return json_string_create(json_string(json)); >> >> case JSON_SERIALIZED_OBJECT: >> return json_serialized_object_create(json); >> @@ -589,7 +589,7 @@ json_hash(const struct json *json, size_t basis) >> >> case JSON_STRING: >> case JSON_SERIALIZED_OBJECT: >> - return hash_string(json->string, basis); >> + return hash_string(json_string(json), basis); > > Hello Ilya, > > I got a SIGABORT here due to json->type != JSON_STRING in json_string(). Good point. I fixed that in the second patch by splitting the cases and always using the pointer for serialized objects. But I need to move these changes to the first patch. I'll wait a bit more for feedback before porting v2. Best regards, Ilya Maximets.
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c index 1f157e46c..eee9646de 100644 --- a/lib/db-ctl-base.c +++ b/lib/db-ctl-base.c @@ -247,15 +247,17 @@ record_id_equals(const union ovsdb_atom *name, enum ovsdb_atomic_type type, const char *record_id) { if (type == OVSDB_TYPE_STRING) { - if (!strcmp(name->s->string, record_id)) { + const char *name_str = json_string(name->s); + + if (!strcmp(name_str, record_id)) { return true; } struct uuid uuid; size_t len = strlen(record_id); if (len >= 4 - && uuid_from_string(&uuid, name->s->string) - && !strncmp(name->s->string, record_id, len)) { + && uuid_from_string(&uuid, name_str) + && !strncmp(name_str, record_id, len)) { return true; } diff --git a/lib/json.c b/lib/json.c index 2649e8e12..20626c445 100644 --- a/lib/json.c +++ b/lib/json.c @@ -493,7 +493,7 @@ json_deep_clone(const struct json *json) return json_deep_clone_array(&json->array); case JSON_STRING: - return json_string_create(json->string); + return json_string_create(json_string(json)); case JSON_SERIALIZED_OBJECT: return json_serialized_object_create(json); @@ -589,7 +589,7 @@ json_hash(const struct json *json, size_t basis) case JSON_STRING: case JSON_SERIALIZED_OBJECT: - return hash_string(json->string, basis); + return hash_string(json_string(json), basis); case JSON_NULL: case JSON_FALSE: @@ -665,7 +665,7 @@ json_equal(const struct json *a, const struct json *b) case JSON_STRING: case JSON_SERIALIZED_OBJECT: - return !strcmp(a->string, b->string); + return !strcmp(json_string(a), json_string(b)); case JSON_NULL: case JSON_FALSE: @@ -1064,7 +1064,7 @@ struct json * json_from_serialized_object(const struct json *json) { ovs_assert(json->type == JSON_SERIALIZED_OBJECT); - return json_from_string(json->string); + return json_from_string(json_string(json)); } /* Reads the file named 'file_name', parses its contents as a JSON object or @@ -1656,11 +1656,11 @@ json_serialize(const struct json *json, struct json_serializer *s) break; case JSON_STRING: - json_serialize_string(json->string, ds); + json_serialize_string(json_string(json), ds); break; case JSON_SERIALIZED_OBJECT: - ds_put_cstr(ds, json->string); + ds_put_cstr(ds, json_string(json)); break; case JSON_N_TYPES: diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c index 90723a42b..f01a2e56f 100644 --- a/lib/jsonrpc.c +++ b/lib/jsonrpc.c @@ -737,7 +737,7 @@ jsonrpc_msg_from_json(struct json *json, struct jsonrpc_msg **msgp) } msg = xzalloc(sizeof *msg); - msg->method = method ? xstrdup(method->string) : NULL; + msg->method = method ? xstrdup(json_string(method)) : NULL; msg->params = null_from_json_null(shash_find_and_delete(object, "params")); msg->result = null_from_json_null(shash_find_and_delete(object, "result")); msg->error = null_from_json_null(shash_find_and_delete(object, "error")); @@ -1204,7 +1204,7 @@ jsonrpc_session_recv(struct jsonrpc_session *s) jsonrpc_session_send(s, reply); } else if (msg->type == JSONRPC_REPLY && msg->id && msg->id->type == JSON_STRING - && !strcmp(msg->id->string, "echo")) { + && !strcmp(json_string(msg->id), "echo")) { /* It's a reply to our echo request. Suppress it. */ } else { return msg; diff --git a/lib/ovsdb-cs.c b/lib/ovsdb-cs.c index 87c6211bf..20386101f 100644 --- a/lib/ovsdb-cs.c +++ b/lib/ovsdb-cs.c @@ -1878,7 +1878,7 @@ server_column_get_string(const struct server_row *row, { ovs_assert(server_columns[index].type.key.type == OVSDB_TYPE_STRING); const struct ovsdb_datum *d = &row->data[index]; - return d->n == 1 ? d->keys[0].s->string : default_value; + return d->n == 1 ? json_string(d->keys[0].s) : default_value; } static bool diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c index e4149401f..d38eee5d0 100644 --- a/lib/ovsdb-data.c +++ b/lib/ovsdb-data.c @@ -264,7 +264,7 @@ unwrap_json(const struct json *json, const char *name, if (json->type != JSON_ARRAY || json->array.n != 2 || json->array.elems[0]->type != JSON_STRING - || (name && strcmp(json->array.elems[0]->string, name)) + || (name && strcmp(json_string(json->array.elems[0]), name)) || json->array.elems[1]->type != value_type) { *value = NULL; @@ -1278,7 +1278,7 @@ ovsdb_datum_from_json__(struct ovsdb_datum *datum, || (json->type == JSON_ARRAY && json->array.n > 0 && json->array.elems[0]->type == JSON_STRING - && !strcmp(json->array.elems[0]->string, "set"))) { + && !strcmp(json_string(json->array.elems[0]), "set"))) { bool is_map = ovsdb_type_is_map(type); const char *class = is_map ? "map" : "set"; const struct json *inner; diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index 226a4b475..cef4a337b 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -2865,8 +2865,8 @@ substitute_uuids(struct json *json, const struct ovsdb_idl_txn *txn) if (json->array.n == 2 && json->array.elems[0]->type == JSON_STRING && json->array.elems[1]->type == JSON_STRING - && !strcmp(json->array.elems[0]->string, "uuid") - && uuid_from_string(&uuid, json->array.elems[1]->string)) { + && !strcmp(json_string(json->array.elems[0]), "uuid") + && uuid_from_string(&uuid, json_string(json->array.elems[1]))) { const struct ovsdb_idl_row *row; row = ovsdb_idl_txn_get_row(txn, &uuid); @@ -4038,18 +4038,20 @@ ovsdb_idl_txn_process_reply(struct ovsdb_idl *idl, error = shash_find_data(json_object(op), "error"); if (error) { if (error->type == JSON_STRING) { - if (!strcmp(error->string, "timed out")) { + const char *error_string = json_string(error); + + if (!strcmp(error_string, "timed out")) { soft_errors++; - } else if (!strcmp(error->string, + } else if (!strcmp(error_string, "unknown database")) { ovsdb_cs_flag_inconsistency(idl->cs); soft_errors++; - } else if (!strcmp(error->string, "not owner")) { + } else if (!strcmp(error_string, "not owner")) { lock_errors++; - } else if (!strcmp(error->string, "not allowed")) { + } else if (!strcmp(error_string, "not allowed")) { hard_errors++; ovsdb_idl_txn_set_error_json(txn, op); - } else if (strcmp(error->string, "aborted")) { + } else if (strcmp(error_string, "aborted")) { hard_errors++; ovsdb_idl_txn_set_error_json(txn, op); VLOG_WARN_RL(&other_rl, diff --git a/lib/ovsdb-parser.c b/lib/ovsdb-parser.c index 38119c195..07a670781 100644 --- a/lib/ovsdb-parser.c +++ b/lib/ovsdb-parser.c @@ -83,7 +83,7 @@ ovsdb_parser_member(struct ovsdb_parser *parser, const char *name, if (((int) value->type >= 0 && value->type < JSON_N_TYPES && types & (1u << value->type)) || (types & OP_ID && value->type == JSON_STRING - && ovsdb_parser_is_id(value->string))) + && ovsdb_parser_is_id(json_string(value)))) { sset_add(&parser->used, name); return value; diff --git a/lib/ovsdb-types.c b/lib/ovsdb-types.c index 69f132597..fce540f4d 100644 --- a/lib/ovsdb-types.c +++ b/lib/ovsdb-types.c @@ -476,7 +476,7 @@ ovsdb_base_type_from_json(struct ovsdb_base_type *base, if (refTable) { const struct json *refType; - base->uuid.refTableName = xstrdup(refTable->string); + base->uuid.refTableName = xstrdup(json_string(refTable)); /* We can't set base->uuid.refTable here because we don't have * enough context (we might not even be running in ovsdb-server). @@ -718,7 +718,7 @@ ovsdb_type_from_json(struct ovsdb_type *type, const struct json *json) } if (max && max->type == JSON_STRING - && !strcmp(max->string, "unlimited")) { + && !strcmp(json_string(max), "unlimited")) { type->n_max = UINT_MAX; } else { error = n_from_json(max, &type->n_max); diff --git a/ovsdb/column.c b/ovsdb/column.c index 9dfc8714f..37cda730c 100644 --- a/ovsdb/column.c +++ b/ovsdb/column.c @@ -173,7 +173,7 @@ ovsdb_column_set_from_json(const struct json *json, goto error; } - s = json->array.elems[i]->string; + s = json_string(json->array.elems[i]); column = shash_find_data(&schema->columns, s); if (!column) { error = ovsdb_syntax_error(json, NULL, "%s is not a valid " diff --git a/ovsdb/execution.c b/ovsdb/execution.c index f4cc9e802..756129da4 100644 --- a/ovsdb/execution.c +++ b/ovsdb/execution.c @@ -128,7 +128,7 @@ ovsdb_execute_compose(struct ovsdb *db, const struct ovsdb_session *session, if (params->type != JSON_ARRAY || !params->array.n || params->array.elems[0]->type != JSON_STRING - || strcmp(params->array.elems[0]->string, db->schema->name)) { + || strcmp(json_string(params->array.elems[0]), db->schema->name)) { if (params->type != JSON_ARRAY) { error = ovsdb_syntax_error(params, NULL, "array expected"); } else { diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c index ba5a4211b..50fb2c7f1 100644 --- a/ovsdb/jsonrpc-server.c +++ b/ovsdb/jsonrpc-server.c @@ -900,7 +900,7 @@ ovsdb_jsonrpc_lookup_db(const struct ovsdb_jsonrpc_session *s, goto error; } - db_name = params->elems[0]->string; + db_name = json_string(params->elems[0]); db = shash_find_data(&s->up.server->dbs, db_name); if (!db) { error = ovsdb_syntax_error( @@ -1447,7 +1447,7 @@ ovsdb_jsonrpc_parse_monitor_request( "array of column names expected"); } - s = columns->array.elems[i]->string; + s = json_string(columns->array.elems[i]); column = shash_find_data(&table->schema->columns, s); if (!column) { return ovsdb_syntax_error(columns, NULL, "%s is not a valid " @@ -1590,7 +1590,7 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session *s, struct ovsdb *db, goto error; } struct uuid txn_uuid; - if (!uuid_from_string(&txn_uuid, last_id->string)) { + if (!uuid_from_string(&txn_uuid, json_string(last_id))) { error = ovsdb_syntax_error(last_id, NULL, "last-txn-id must be UUID format."); goto error; diff --git a/ovsdb/log.c b/ovsdb/log.c index fff7c6ba1..0c3577119 100644 --- a/ovsdb/log.c +++ b/ovsdb/log.c @@ -495,7 +495,7 @@ ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp) "offset %lld are not valid JSON (%s)", file->display_name, data_length, (long long int) data_offset, - json->string); + json_string(json)); goto error; } if (json->type != JSON_OBJECT) { diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c index 5258eccaa..f85057b6c 100644 --- a/ovsdb/ovsdb-client.c +++ b/ovsdb/ovsdb-client.c @@ -501,7 +501,7 @@ parse_json(const char *s) { struct json *json = json_from_string(s); if (json->type == JSON_STRING) { - ovs_fatal(0, "\"%s\": %s", s, json->string); + ovs_fatal(0, "\"%s\": %s", s, json_string(json)); } return json; } @@ -596,7 +596,7 @@ fetch_dbs(struct jsonrpc *rpc, struct svec *dbs) if (name->type != JSON_STRING) { ovs_fatal(0, "list_dbs response %"PRIuSIZE" is not string", i); } - svec_add(dbs, name->string); + svec_add(dbs, json_string(name)); } jsonrpc_msg_destroy(reply); svec_sort(dbs); diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in index 9a54f06a1..5c3e76c77 100755 --- a/ovsdb/ovsdb-idlc.in +++ b/ovsdb/ovsdb-idlc.in @@ -601,13 +601,17 @@ static void elif (type.n_min == 1 and type.n_max == 1) or type.is_optional_pointer(): print("") print(" if (datum->n >= 1) {") - if not type.key.ref_table: + if type.key.type == ovs.db.types.StringType: + print(" %s = CONST_CAST(char *, json_string(datum->keys[0].s));" % keyVar) + elif not type.key.ref_table: print(" %s = datum->keys[0].%s;" % (keyVar, type.key.type.to_rvalue_string())) else: print(" %s = %s%s_cast(ovsdb_idl_get_row_arc(row_, &%stable_%s, &datum->keys[0].uuid));" % (keyVar, prefix, type.key.ref_table.name.lower(), prefix, type.key.ref_table.name.lower())) if valueVar: - if not type.value.ref_table: + if type.value.type == ovs.db.types.StringType: + print(" %s = CONST_CAST(char *, json_string(datum->values[0].s));" % valueVar) + elif not type.value.ref_table: print(" %s = datum->values[0].%s;" % (valueVar, type.value.type.to_rvalue_string())) else: print(" %s = %s%s_cast(ovsdb_idl_get_row_arc(row_, &%stable_%s, &datum->values[0].uuid));" % (valueVar, prefix, type.value.ref_table.name.lower(), prefix, type.value.ref_table.name.lower())) @@ -635,6 +639,8 @@ static void }\ """ % (prefix, type.key.ref_table.name.lower(), prefix, type.key.ref_table.name.lower(), prefix, type.key.ref_table.name.lower())) keySrc = "keyRow" + elif type.key.type == ovs.db.types.StringType: + keySrc = "CONST_CAST(char *, json_string(datum->keys[i].s))" else: keySrc = "datum->keys[i].%s" % type.key.type.to_rvalue_string() if type.value and type.value.ref_table: @@ -645,6 +651,8 @@ static void }\ """ % (prefix, type.value.ref_table.name.lower(), prefix, type.value.ref_table.name.lower(), prefix, type.value.ref_table.name.lower())) valueSrc = "valueRow" + elif valueVar and type.value.type == ovs.db.types.StringType: + valueSrc = "CONST_CAST(char *, json_string(datum->values[i].s))" elif valueVar: valueSrc = "datum->values[i].%s" % type.value.type.to_rvalue_string() print(" if (!row->n_%s) {" % (columnName)) diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c index 354382f11..9051020b3 100644 --- a/ovsdb/ovsdb-tool.c +++ b/ovsdb/ovsdb-tool.c @@ -229,7 +229,7 @@ parse_json(const char *s) { struct json *json = json_from_string(s); if (json->type == JSON_STRING) { - ovs_fatal(0, "\"%s\": %s", s, json->string); + ovs_fatal(0, "\"%s\": %s", s, json_string(json)); } return json; } diff --git a/ovsdb/ovsdb-util.c b/ovsdb/ovsdb-util.c index ec4537890..c15e6bb64 100644 --- a/ovsdb/ovsdb-util.c +++ b/ovsdb/ovsdb-util.c @@ -111,13 +111,13 @@ ovsdb_util_read_map_string_column(const struct ovsdb_row *row, for (i = 0; i < datum->n; i++) { atom_key = &datum->keys[i]; - if (!strcmp(atom_key->s->string, key)) { + if (!strcmp(json_string(atom_key->s), key)) { atom_value = &datum->values[i]; break; } } - return atom_value ? atom_value->s->string : NULL; + return atom_value ? json_string(atom_value->s) : NULL; } /* Read string-uuid key-values from a map. Returns the row associated with @@ -143,7 +143,7 @@ ovsdb_util_read_map_string_uuid_column(const struct ovsdb_row *row, const struct ovsdb_datum *datum = &row->fields[column->index]; for (size_t i = 0; i < datum->n; i++) { union ovsdb_atom *atom_key = &datum->keys[i]; - if (!strcmp(atom_key->s->string, key)) { + if (!strcmp(json_string(atom_key->s), key)) { const union ovsdb_atom *atom_value = &datum->values[i]; return ovsdb_table_get_row(ref_table, &atom_value->uuid); } @@ -181,7 +181,7 @@ ovsdb_util_read_string_column(const struct ovsdb_row *row, const union ovsdb_atom *atom; atom = ovsdb_util_read_column(row, column_name, OVSDB_TYPE_STRING); - *stringp = atom ? atom->s->string : NULL; + *stringp = atom ? json_string(atom->s) : NULL; return atom != NULL; } diff --git a/ovsdb/replication.c b/ovsdb/replication.c index 56720cb10..67eab9c42 100644 --- a/ovsdb/replication.c +++ b/ovsdb/replication.c @@ -220,7 +220,7 @@ replication_run_db(struct replication_db *rdb) if (msg->params->type == JSON_ARRAY && msg->params->array.n == 2 && msg->params->array.elems[0]->type == JSON_STRING) { - char *db_name = msg->params->array.elems[0]->string; + const char *db_name = json_string(msg->params->array.elems[0]); if (!strcmp(db_name, rdb->db->name)) { struct ovsdb_error *error; diff --git a/python/ovs/_json.c b/python/ovs/_json.c index afa97896d..5f4388f80 100644 --- a/python/ovs/_json.c +++ b/python/ovs/_json.c @@ -135,7 +135,7 @@ json_to_python(struct json *json) return PyLong_FromLong((long) json->integer); case JSON_STRING: - return PyUnicode_FromString(json->string); + return PyUnicode_FromString(json_string(json)); default: return NULL; } diff --git a/python/ovs/db/types.py b/python/ovs/db/types.py index 3318a3b6f..af6aa878c 100644 --- a/python/ovs/db/types.py +++ b/python/ovs/db/types.py @@ -50,7 +50,7 @@ class AtomicType(object): def to_rvalue_string(self): if self == StringType: - return 's->' + self.name + raise error.Error("can't convert string type to rvalue") return self.name def to_lvalue_string(self): diff --git a/tests/test-json.c b/tests/test-json.c index 6cf5eb75d..2ba92d119 100644 --- a/tests/test-json.c +++ b/tests/test-json.c @@ -101,8 +101,8 @@ test_json_equal(const struct json *a, const struct json *b, case JSON_STRING: case JSON_SERIALIZED_OBJECT: - ovs_assert(a->string != b->string); - ovs_assert(!strcmp(a->string, b->string)); + ovs_assert(json_string(a) != json_string(b)); + ovs_assert(!strcmp(json_string(a), json_string(b))); return; case JSON_NULL: @@ -154,7 +154,7 @@ print_test_and_free_json(struct json *json) { bool ok; if (json->type == JSON_STRING) { - printf("error: %s\n", json->string); + printf("error: %s\n", json_string(json)); ok = false; } else { char *s = json_to_string(json, JSSF_SORT | (pretty ? JSSF_PRETTY : 0)); diff --git a/tests/test-jsonrpc.c b/tests/test-jsonrpc.c index 04e941b14..02504c289 100644 --- a/tests/test-jsonrpc.c +++ b/tests/test-jsonrpc.c @@ -124,7 +124,7 @@ parse_json(const char *s) { struct json *json = json_from_string(s); if (json->type == JSON_STRING) { - ovs_fatal(0, "\"%s\": %s", s, json->string); + ovs_fatal(0, "\"%s\": %s", s, json_string(json)); } return json; } diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index 52438e677..0511e4170 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -265,7 +265,7 @@ parse_json(const char *s) { struct json *json = json_from_string(s); if (json->type == JSON_STRING) { - ovs_fatal(0, "\"%s\": %s", s, json->string); + ovs_fatal(0, "\"%s\": %s", s, json_string(json)); } return json; } @@ -937,7 +937,7 @@ do_compare_rows(struct ovs_cmdl_context *ctx) ovs_fatal(0, "\"%s\" does not have expected form " "[\"name\", {data}]", ctx->argv[i]); } - names[i] = xstrdup(json->array.elems[0]->string); + names[i] = xstrdup(json_string(json->array.elems[0])); check_ovsdb_error(ovsdb_row_from_json(rows[i], json->array.elems[1], NULL, NULL, false)); json_destroy(json); @@ -2385,7 +2385,8 @@ parse_uuids(const struct json *json, struct ovsdb_symbol_table *symtab, { struct uuid uuid; - if (json->type == JSON_STRING && uuid_from_string(&uuid, json->string)) { + if (json->type == JSON_STRING + && uuid_from_string(&uuid, json_string(json))) { char *name = xasprintf("#%"PRIuSIZE"#", *n); fprintf(stderr, "%s = "UUID_FMT"\n", name, UUID_ARGS(&uuid)); ovsdb_symbol_table_put(symtab, name, &uuid, false);
We'll be changing the way strings are stored, so the direct access will not be safe anymore. Change all the users to use the proper API as they should have been doing anyway. The only code outside of json implementation for which direct access is preserved is substitute_uuids() in test-ovsdb.c. It's an unusual string manipulation that is only needed for the testing, so doesn't seem worthy adding a new API function. We could introduce something like json_string_replace() if this use case will appear somewhere else in the future. Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- lib/db-ctl-base.c | 8 +++++--- lib/json.c | 12 ++++++------ lib/jsonrpc.c | 4 ++-- lib/ovsdb-cs.c | 2 +- lib/ovsdb-data.c | 4 ++-- lib/ovsdb-idl.c | 16 +++++++++------- lib/ovsdb-parser.c | 2 +- lib/ovsdb-types.c | 4 ++-- ovsdb/column.c | 2 +- ovsdb/execution.c | 2 +- ovsdb/jsonrpc-server.c | 6 +++--- ovsdb/log.c | 2 +- ovsdb/ovsdb-client.c | 4 ++-- ovsdb/ovsdb-idlc.in | 12 ++++++++++-- ovsdb/ovsdb-tool.c | 2 +- ovsdb/ovsdb-util.c | 8 ++++---- ovsdb/replication.c | 2 +- python/ovs/_json.c | 2 +- python/ovs/db/types.py | 2 +- tests/test-json.c | 6 +++--- tests/test-jsonrpc.c | 2 +- tests/test-ovsdb.c | 7 ++++--- 22 files changed, 62 insertions(+), 49 deletions(-)