refactor: property_node attribute handling

The previous implementation just went along with how AVS
property models and deals with attributes: attributes are just
another type of node that follow the same rules as standard
xml nodes/elements.

This design already showed several flaws in application as
these nodes always had to be referred to with an appended
‘@‘ on the attribute keys. This was mitigated with an “ext”
function that hides the whole details of “find the attribute node
before reading the attribute” steps.

By adding another property_node implementation with mxml,
the AVS style abstract layer showed incompatibilities with
the “an attribute is a node” approach. mxml doesn’t treat them as a type of node and just addresses them directly using their keys. This is lot simpler and aligns with how handling
attribute is done throughout the code thus far.

Refactor the property_node interface and the current AVS
implementation to also adapt this. Hide the detail that AVS
treats attributes as nodes and also the whole “append @“
to the keys notation.
This commit is contained in:
icex2 2024-08-15 11:34:31 +02:00
parent 8d98a38519
commit ee8f2a05fe
8 changed files with 134 additions and 106 deletions

View File

@ -933,15 +933,12 @@ static core_property_node_result_t _avs_ext_property_node_double_create(
static core_property_node_result_t _avs_ext_property_node_attr_create(
const core_property_node_t *parent_node_,
const char *key,
const char *value,
core_property_node_t *node_out_)
const char *value)
{
avs_ext_property_internal_node_t *parent_node;
avs_ext_property_internal_node_t *node_out;
struct property_node *tmp;
parent_node = (avs_ext_property_internal_node_t *) parent_node_;
node_out = (avs_ext_property_internal_node_t *) node_out_;
tmp = property_node_create(
parent_node->property->property,
@ -954,13 +951,6 @@ static core_property_node_result_t _avs_ext_property_node_attr_create(
return CORE_PROPERTY_NODE_RESULT_ERROR_INTERNAL;
}
if (node_out) {
memset(node_out, 0, sizeof(avs_ext_property_internal_node_t));
node_out->property = parent_node->property;
node_out->node = tmp;
}
return CORE_PROPERTY_NODE_RESULT_SUCCESS;
}
@ -1233,15 +1223,32 @@ static core_property_node_result_t _avs_ext_property_node_double_read(
}
static core_property_node_result_t _avs_ext_property_node_attr_read(
const core_property_node_t *parent_node_, char *value, size_t len)
const core_property_node_t *parent_node_, const char *key, char *value, size_t len)
{
avs_ext_property_internal_node_t *parent_node;
avs_error error;
size_t attr_key_len;
char *attr_key;
struct property_node *node_attr;
parent_node = (avs_ext_property_internal_node_t *) parent_node_;
// Append @ denoting an attribute, plus null terminator
attr_key_len = strlen(key) + 1 + 1;
attr_key = xmalloc(sizeof(char) * attr_key_len);
str_cpy(attr_key, attr_key_len, key);
str_cat(attr_key, attr_key_len, "@");
node_attr = property_search(NULL, parent_node->node, attr_key);
if (node_attr == NULL) {
return CORE_PROPERTY_NODE_RESULT_NODE_NOT_FOUND;
}
error =
property_node_read(parent_node->node, PROPERTY_TYPE_ATTR, value, len);
property_node_read(node_attr, PROPERTY_TYPE_ATTR, value, len);
if (AVS_IS_ERROR(error)) {
return CORE_PROPERTY_NODE_RESULT_ERROR_INTERNAL;
@ -1268,6 +1275,41 @@ static core_property_node_result_t _avs_ext_property_node_bool_read(
return CORE_PROPERTY_NODE_RESULT_SUCCESS;
}
static core_property_node_result_t _avs_ext_property_node_attr_remove(
const core_property_node_t *parent_node_, const char *key)
{
avs_ext_property_internal_node_t *parent_node;
struct property_node *attr_node;
size_t attr_key_len;
char *attr_key;
avs_error error;
parent_node = (avs_ext_property_internal_node_t *) parent_node_;
// Append @ denoting an attribute, plus null terminator
attr_key_len = strlen(key) + 1 + 1;
attr_key = xmalloc(sizeof(char) * attr_key_len);
str_cpy(attr_key, attr_key_len, key);
str_cat(attr_key, attr_key_len, "@");
// AVS property treats attributes as a type of node
attr_node = property_search(NULL, parent_node->node, attr_key);
if (attr_node == NULL) {
return CORE_PROPERTY_NODE_RESULT_NODE_NOT_FOUND;
}
error = property_node_remove(attr_node);
if (AVS_IS_ERROR(error)) {
return CORE_PROPERTY_NODE_RESULT_ERROR_INTERNAL;
}
return CORE_PROPERTY_NODE_RESULT_SUCCESS;
}
static core_property_node_result_t
_avs_ext_property_node_remove(const core_property_node_t *node_)
{
@ -1350,6 +1392,7 @@ void avs_ext_property_node_core_api_get(core_property_node_api_t *api)
api->v1.attr_read = _avs_ext_property_node_attr_read;
api->v1.bool_read = _avs_ext_property_node_bool_read;
api->v1.remove = _avs_ext_property_node_remove;
api->v1.attr_remove = _avs_ext_property_node_attr_remove;
api->v1.copy = _avs_ext_property_node_copy;
}

View File

@ -187,26 +187,6 @@ core_property_node_result_t core_property_node_ext_str_read(
return core_property_node_str_read(&tmp, value, len);
}
core_property_node_result_t core_property_node_ext_attr_read(
const core_property_node_t *node, const char *name, char *value, size_t len)
{
core_property_node_t tmp;
core_property_node_result_t result;
log_assert(node);
log_assert(name);
log_assert(value);
log_assert(len > 0);
result = core_property_node_search(node, name, &tmp);
if (CORE_PROPERTY_NODE_RESULT_IS_ERROR(result)) {
return result;
}
return core_property_node_attr_read(&tmp, value, len);
}
core_property_node_result_t core_property_node_ext_u8_read_or_default(
const core_property_node_t *node,
const char *name,
@ -458,27 +438,21 @@ core_property_node_result_t core_property_node_ext_bool_replace(
core_property_node_result_t core_property_node_ext_attr_replace(
core_property_node_t *node, const char *name, const char *val)
{
core_property_node_t tmp;
core_property_node_result_t result;
log_assert(node);
log_assert(name);
log_assert(val);
result = core_property_node_search(node, name, &tmp);
result = core_property_node_attr_remove(node, name);
if (result != CORE_PROPERTY_NODE_RESULT_NODE_NOT_FOUND) {
if (CORE_PROPERTY_NODE_RESULT_IS_ERROR(result)) {
return result;
}
result = core_property_node_remove(&tmp);
if (CORE_PROPERTY_NODE_RESULT_IS_ERROR(result)) {
return result;
}
}
return core_property_node_attr_create(node, name, val, NULL);
return core_property_node_attr_create(node, name, val);
}
core_property_node_result_t core_property_node_ext_extract(

View File

@ -457,16 +457,15 @@ static core_property_node_result_t _core_property_node_trace_double_create(
static core_property_node_result_t _core_property_node_trace_attr_create(
const core_property_node_t *parent_node,
const char *key,
const char *value,
core_property_node_t *node_out)
const char *value)
{
core_property_node_result_t result;
log_misc(
">>> attr_create(%p, %s, %s, %p)", parent_node, key, value, node_out);
">>> attr_create(%p, %s, %s, %p)", parent_node, key, value);
result = _core_property_node_trace_target_api.v1.attr_create(
parent_node, key, value, node_out);
parent_node, key, value);
log_misc(
"<<< attr_create(%p, %s): %s",
@ -747,14 +746,14 @@ static core_property_node_result_t _core_property_node_trace_double_read(
}
static core_property_node_result_t _core_property_node_trace_attr_read(
const core_property_node_t *parent_node, char *value, size_t len)
const core_property_node_t *parent_node, const char *key, char *value, size_t len)
{
core_property_node_result_t result;
log_misc(">>> attr_read(%p, %p, %d)", parent_node, value, len);
log_misc(">>> attr_read(%p, %s, %p, %d)", parent_node, key, value, len);
result = _core_property_node_trace_target_api.v1.attr_read(
parent_node, value, len);
parent_node, key, value, len);
log_misc(
"<<< attr_read(%p, %s): %s",

View File

@ -409,8 +409,7 @@ core_property_node_result_t core_property_node_double_create(
core_property_node_result_t core_property_node_attr_create(
const core_property_node_t *parent_node,
const char *key,
const char *value,
core_property_node_t *node_out)
const char *value)
{
log_assert(_core_property_node_api_is_valid());
log_assert(parent_node);
@ -418,7 +417,7 @@ core_property_node_result_t core_property_node_attr_create(
log_assert(value);
return _core_property_node_api.v1.attr_create(
parent_node, key, value, node_out);
parent_node, key, value);
}
core_property_node_result_t core_property_node_bool_create(
@ -566,13 +565,14 @@ core_property_node_result_t core_property_node_double_read(
}
core_property_node_result_t core_property_node_attr_read(
const core_property_node_t *parent_node, char *value, size_t len)
const core_property_node_t *parent_node, const char *key, char *value, size_t len)
{
log_assert(_core_property_node_api_is_valid());
log_assert(parent_node);
log_assert(key);
log_assert(value);
return _core_property_node_api.v1.attr_read(parent_node, value, len);
return _core_property_node_api.v1.attr_read(parent_node, key, value, len);
}
core_property_node_result_t core_property_node_bool_read(
@ -594,6 +594,16 @@ core_property_node_remove(const core_property_node_t *node)
return _core_property_node_api.v1.remove(node);
}
core_property_node_result_t core_property_node_attr_remove(
const core_property_node_t *parent_node, const char *key)
{
log_assert(_core_property_node_api_is_valid());
log_assert(parent_node);
log_assert(key);
return _core_property_node_api.v1.attr_remove(parent_node, key);
}
core_property_node_result_t core_property_node_copy(
core_property_node_t *dst_node, const core_property_node_t *src_node)
{

View File

@ -115,8 +115,7 @@ typedef core_property_node_result_t (*core_property_node_double_create_t)(
typedef core_property_node_result_t (*core_property_node_attr_create_t)(
const core_property_node_t *parent_node,
const char *key,
const char *value,
core_property_node_t *node_out);
const char *value);
typedef core_property_node_result_t (*core_property_node_bool_create_t)(
const core_property_node_t *parent_node,
const char *key,
@ -149,11 +148,13 @@ typedef core_property_node_result_t (*core_property_node_float_read_t)(
typedef core_property_node_result_t (*core_property_node_double_read_t)(
const core_property_node_t *parent_node, double *value);
typedef core_property_node_result_t (*core_property_node_attr_read_t)(
const core_property_node_t *parent_node, char *value, size_t len);
const core_property_node_t *parent_node, const char *key, char *value, size_t len);
typedef core_property_node_result_t (*core_property_node_bool_read_t)(
const core_property_node_t *parent_node, bool *value);
typedef core_property_node_result_t (*core_property_node_remove_t)(
const core_property_node_t *node);
typedef core_property_node_result_t (*core_property_node_attr_remove_t)(
const core_property_node_t *parent_node, const char *key);
typedef core_property_node_result_t (*core_property_node_copy_t)(
core_property_node_t *dst_node, const core_property_node_t *src_node);
@ -200,6 +201,7 @@ typedef struct core_property_node_api {
core_property_node_attr_read_t attr_read;
core_property_node_bool_read_t bool_read;
core_property_node_remove_t remove;
core_property_node_attr_remove_t attr_remove;
core_property_node_copy_t copy;
} v1;
} core_property_node_api_t;
@ -302,8 +304,7 @@ core_property_node_result_t core_property_node_double_create(
core_property_node_result_t core_property_node_attr_create(
const core_property_node_t *parent_node,
const char *key,
const char *value,
core_property_node_t *node_out);
const char *value);
core_property_node_result_t core_property_node_bool_create(
const core_property_node_t *parent_node,
const char *key,
@ -336,11 +337,13 @@ core_property_node_result_t core_property_node_float_read(
core_property_node_result_t core_property_node_double_read(
const core_property_node_t *parent_node, double *value);
core_property_node_result_t core_property_node_attr_read(
const core_property_node_t *parent_node, char *value, size_t len);
const core_property_node_t *parent_node, const char *key, char *value, size_t len);
core_property_node_result_t core_property_node_bool_read(
const core_property_node_t *parent_node, bool *value);
core_property_node_result_t
core_property_node_remove(const core_property_node_t *node);
core_property_node_result_t core_property_node_attr_remove(
const core_property_node_t *parent_node, const char *key);
core_property_node_result_t core_property_node_copy(
core_property_node_t *dst_node, const core_property_node_t *src_node);

View File

@ -33,24 +33,24 @@ static void _avs_config_node_vfs_copy(
// Ignore errors and default to empty
memset(data, 0, sizeof(data));
core_property_node_ext_attr_read(source, "name@", data, sizeof(data));
core_property_node_ext_attr_replace(parent, "name@", data);
core_property_node_attr_read(source, "name", data, sizeof(data));
core_property_node_ext_attr_replace(parent, "name", data);
memset(data, 0, sizeof(data));
core_property_node_ext_attr_read(source, "fstype@", data, sizeof(data));
core_property_node_ext_attr_replace(parent, "fstype@", data);
core_property_node_attr_read(source, "fstype", data, sizeof(data));
core_property_node_ext_attr_replace(parent, "fstype", data);
memset(data, 0, sizeof(data));
core_property_node_ext_attr_read(source, "src@", data, sizeof(data));
core_property_node_ext_attr_replace(parent, "src@", data);
core_property_node_attr_read(source, "src", data, sizeof(data));
core_property_node_ext_attr_replace(parent, "src", data);
memset(data, 0, sizeof(data));
core_property_node_ext_attr_read(source, "dst@", data, sizeof(data));
core_property_node_ext_attr_replace(parent, "dst@", data);
core_property_node_attr_read(source, "dst", data, sizeof(data));
core_property_node_ext_attr_replace(parent, "dst", data);
memset(data, 0, sizeof(data));
core_property_node_ext_attr_read(source, "opt@", data, sizeof(data));
core_property_node_ext_attr_replace(parent, "opt@", data);
core_property_node_attr_read(source, "opt", data, sizeof(data));
core_property_node_ext_attr_replace(parent, "opt", data);
}
static core_property_node_result_t
@ -113,9 +113,9 @@ _avs_config_mounttable_vfs_nodes_merge_strategy_do(
core_property_node_fatal_on_error(result);
if (str_eq(parent_child_name, "vfs")) {
result = core_property_node_ext_attr_read(
result = core_property_node_attr_read(
&source_child,
"name@",
"name",
name_source,
sizeof(name_source));
@ -125,8 +125,8 @@ _avs_config_mounttable_vfs_nodes_merge_strategy_do(
"vfs source node");
}
result = core_property_node_ext_attr_read(
&source_child, "dst@", dst_source, sizeof(dst_source));
result = core_property_node_attr_read(
&source_child, "dst", dst_source, sizeof(dst_source));
if (CORE_PROPERTY_NODE_RESULT_IS_ERROR(result)) {
log_fatal(
@ -134,9 +134,9 @@ _avs_config_mounttable_vfs_nodes_merge_strategy_do(
"vfs source node");
}
result = core_property_node_ext_attr_read(
result = core_property_node_attr_read(
&parent_child,
"name@",
"name",
name_parent,
sizeof(name_parent));
@ -146,8 +146,8 @@ _avs_config_mounttable_vfs_nodes_merge_strategy_do(
"vfs parent node");
}
result = core_property_node_ext_attr_read(
&parent_child, "dst@", dst_parent, sizeof(dst_parent));
result = core_property_node_attr_read(
&parent_child, "dst", dst_parent, sizeof(dst_parent));
if (CORE_PROPERTY_NODE_RESULT_IS_ERROR(result)) {
log_fatal(
@ -591,7 +591,6 @@ void avs_config_local_fs_path_dev_nvram_and_raw_set(
core_property_node_t fs_node;
core_property_node_t mounttable_node;
core_property_node_t vfs_node;
core_property_node_t tmp_node;
core_property_node_result_t result;
log_assert(node);
@ -629,19 +628,19 @@ void avs_config_local_fs_path_dev_nvram_and_raw_set(
core_property_node_fatal_on_error(result);
result = core_property_node_attr_create(
&vfs_node, "name", "boot", &tmp_node);
&vfs_node, "name", "boot");
core_property_node_fatal_on_error(result);
result = core_property_node_attr_create(
&vfs_node, "fstype", "fs", &tmp_node);
&vfs_node, "fstype", "fs");
core_property_node_fatal_on_error(result);
result = core_property_node_attr_create(
&vfs_node, "src", path_dev_raw, &tmp_node);
&vfs_node, "src", path_dev_raw);
core_property_node_fatal_on_error(result);
result = core_property_node_attr_create(
&vfs_node, "dest", "/dev/raw", &tmp_node);
&vfs_node, "dest", "/dev/raw");
core_property_node_fatal_on_error(result);
result = core_property_node_attr_create(
&vfs_node, "opt", "vf=1,posix=1", &tmp_node);
&vfs_node, "opt", "vf=1,posix=1");
core_property_node_fatal_on_error(result);
result =
@ -649,19 +648,19 @@ void avs_config_local_fs_path_dev_nvram_and_raw_set(
core_property_node_fatal_on_error(result);
result = core_property_node_attr_create(
&vfs_node, "name", "boot", &tmp_node);
&vfs_node, "name", "boot");
core_property_node_fatal_on_error(result);
result = core_property_node_attr_create(
&vfs_node, "fstype", "fs", &tmp_node);
&vfs_node, "fstype", "fs");
core_property_node_fatal_on_error(result);
result = core_property_node_attr_create(
&vfs_node, "src", path_dev_nvram, &tmp_node);
&vfs_node, "src", path_dev_nvram);
core_property_node_fatal_on_error(result);
result = core_property_node_attr_create(
&vfs_node, "dest", "/dev/nvram", &tmp_node);
&vfs_node, "dest", "/dev/nvram");
core_property_node_fatal_on_error(result);
result = core_property_node_attr_create(
&vfs_node, "opt", "vf=1,posix=1", &tmp_node);
&vfs_node, "opt", "vf=1,posix=1");
core_property_node_fatal_on_error(result);
} else if (result == CORE_PROPERTY_NODE_RESULT_NODE_NOT_FOUND) {
result = core_property_node_ext_str_replace(
@ -759,8 +758,8 @@ void avs_config_vfs_mounttable_get(
break;
}
result = core_property_node_ext_attr_read(
&cur, "name@", name, sizeof(name));
result = core_property_node_attr_read(
&cur, "name", name, sizeof(name));
if (result == CORE_PROPERTY_NODE_RESULT_NODE_NOT_FOUND) {
log_fatal("Missing 'name' attribute on vfs node");
@ -769,9 +768,9 @@ void avs_config_vfs_mounttable_get(
}
if (str_eq(name, mounttable_selector)) {
result = core_property_node_ext_attr_read(
result = core_property_node_attr_read(
&cur,
"fstype@",
"fstype",
mounttable->entry[pos].fstype,
sizeof(mounttable->entry[pos].fstype));
@ -785,9 +784,9 @@ void avs_config_vfs_mounttable_get(
core_property_node_fatal_on_error(result);
}
result = core_property_node_ext_attr_read(
result = core_property_node_attr_read(
&cur,
"src@",
"src",
mounttable->entry[pos].src,
sizeof(mounttable->entry[pos].src));
@ -799,9 +798,9 @@ void avs_config_vfs_mounttable_get(
core_property_node_fatal_on_error(result);
}
result = core_property_node_ext_attr_read(
result = core_property_node_attr_read(
&cur,
"dst@",
"dst",
mounttable->entry[pos].dst,
sizeof(mounttable->entry[pos].dst));
@ -813,9 +812,9 @@ void avs_config_vfs_mounttable_get(
core_property_node_fatal_on_error(result);
}
result = core_property_node_ext_attr_read(
result = core_property_node_attr_read(
&cur,
"opt@",
"opt",
mounttable->entry[pos].opt,
sizeof(mounttable->entry[pos].opt));

View File

@ -128,9 +128,9 @@ static void _bootstrap_config_inheritance_resolve(
sizeof(core_property_node_t));
while (true) {
result = core_property_node_ext_attr_read(
result = core_property_node_attr_read(
&startup_parent_node,
"inherit@",
"inherit",
inherit_name,
sizeof(inherit_name));
@ -233,8 +233,8 @@ static void _bootstrap_config_load_bootstrap_default_files_config(
break;
}
result = core_property_node_ext_attr_read(
&child, "src@", config->file[i].src, sizeof(config->file[i].src));
result = core_property_node_attr_read(
&child, "src", config->file[i].src, sizeof(config->file[i].src));
if (result == CORE_PROPERTY_NODE_RESULT_NODE_NOT_FOUND) {
log_fatal(
@ -244,8 +244,8 @@ static void _bootstrap_config_load_bootstrap_default_files_config(
core_property_node_fatal_on_error(result);
}
result = core_property_node_ext_attr_read(
&child, "dst@", config->file[i].dst, sizeof(config->file[i].dst));
result = core_property_node_attr_read(
&child, "dst", config->file[i].dst, sizeof(config->file[i].dst));
if (result == CORE_PROPERTY_NODE_RESULT_NODE_NOT_FOUND) {
log_fatal(

View File

@ -53,7 +53,7 @@ _launcher_config_layered_config_nodes_load(const core_property_node_t *node)
}
result =
core_property_node_ext_attr_read(&cur, "kind@", kind, sizeof(kind));
core_property_node_attr_read(&cur, "kind", kind, sizeof(kind));
if (CORE_PROPERTY_NODE_RESULT_IS_ERROR(result)) {
log_fatal("Failed reading 'kind' attribute value of config node");