@@ -267,22 +267,6 @@ uci_free_package(struct uci_package **package)
*package = NULL;
}
-static void
-uci_free_any(struct uci_element **e)
-{
- switch((*e)->type) {
- case UCI_TYPE_SECTION:
- uci_free_section(uci_to_section(*e));
- break;
- case UCI_TYPE_OPTION:
- uci_free_option(uci_to_option(*e));
- break;
- default:
- break;
- }
- *e = NULL;
-}
-
__private struct uci_element *
uci_lookup_list(struct uci_list *list, const char *name)
{
@@ -500,7 +484,7 @@ int uci_rename(struct uci_context *ctx, struct uci_ptr *ptr)
UCI_ASSERT(ctx, ptr->value);
if (!internal && p->has_delta)
- uci_add_delta(ctx, &p->delta, UCI_CMD_RENAME, ptr->section, ptr->option, ptr->value);
+ uci_add_delta(ctx, &p->delta, UCI_CMD_RENAME, ptr->s->e.name, ptr->o ? ptr->o->e.name : NULL, ptr->value);
n = uci_strdup(ctx, ptr->value);
free(e->name);
@@ -552,25 +536,25 @@ int uci_delete(struct uci_context *ctx, struct uci_ptr *ptr)
/* NB: pass on internal flag to uci_del_element */
bool internal = ctx && ctx->internal;
struct uci_package *p;
- struct uci_element *e1, *e2, *tmp;
+ struct uci_element *e, *tmp;
int index;
UCI_HANDLE_ERR(ctx);
- e1 = uci_expand_ptr(ctx, ptr, true);
+ uci_expand_ptr(ctx, ptr, true);
p = ptr->p;
UCI_ASSERT(ctx, ptr->s);
- if (ptr->o && ptr->o->type == UCI_TYPE_LIST && ptr->value && *ptr->value) {
+ if (ptr->o && ptr->o->type == UCI_TYPE_LIST && ptr->value && ptr->value[0]) {
if (!sscanf(ptr->value, "%d", &index))
return 1;
- uci_foreach_element_safe(&ptr->o->v.list, tmp, e2) {
+ uci_foreach_element_safe(&ptr->o->v.list, tmp, e) {
if (index == 0) {
if (!internal && p->has_delta)
- uci_add_delta(ctx, &p->delta, UCI_CMD_REMOVE, ptr->section, ptr->option, ptr->value);
- uci_free_option(uci_to_option(e2));
+ uci_add_delta(ctx, &p->delta, UCI_CMD_REMOVE, ptr->s->e.name, ptr->o->e.name, ptr->value);
+ uci_free_option(uci_to_option(e));
return 0;
}
index--;
@@ -580,14 +564,19 @@ int uci_delete(struct uci_context *ctx, struct uci_ptr *ptr)
}
if (!internal && p->has_delta)
- uci_add_delta(ctx, &p->delta, UCI_CMD_REMOVE, ptr->section, ptr->option, NULL);
+ uci_add_delta(ctx, &p->delta, UCI_CMD_REMOVE, ptr->s->e.name, ptr->o ? ptr->o->e.name : NULL, NULL);
- uci_free_any(&e1);
-
- if (ptr->option)
+ if (ptr->o) {
+ if (ptr->option == ptr->o->e.name)
+ ptr->option = NULL;
+ uci_free_option(ptr->o);
ptr->o = NULL;
- else if (ptr->section)
+ } else {
+ if (ptr->section == ptr->s->e.name)
+ ptr->section = NULL;
+ uci_free_section(ptr->s);
ptr->s = NULL;
+ }
return 0;
}
@@ -622,7 +611,7 @@ int uci_add_list(struct uci_context *ctx, struct uci_ptr *ptr)
struct uci_option *old = ptr->o;
UCI_TRAP_SAVE(ctx, error);
e2 = uci_alloc_generic(ctx, UCI_TYPE_ITEM, old->v.string, sizeof(struct uci_option));
- ptr->o = uci_alloc_list(ptr->s, ptr->option, &old->e.list);
+ ptr->o = uci_alloc_list(ptr->s, old->e.name, &old->e.list);
UCI_TRAP_RESTORE(ctx);
uci_list_add(&ptr->o->v.list, &e2->list);
@@ -637,7 +626,7 @@ int uci_add_list(struct uci_context *ctx, struct uci_ptr *ptr)
uci_list_add(&ptr->o->v.list, &e1->list);
if (!internal && ptr->p->has_delta)
- uci_add_delta(ctx, &ptr->p->delta, UCI_CMD_LIST_ADD, ptr->section, ptr->option, ptr->value);
+ uci_add_delta(ctx, &ptr->p->delta, UCI_CMD_LIST_ADD, ptr->s->e.name, ptr->o->e.name, ptr->value);
return 0;
error:
@@ -661,7 +650,7 @@ int uci_del_list(struct uci_context *ctx, struct uci_ptr *ptr)
UCI_ASSERT(ctx, ptr->s);
UCI_ASSERT(ctx, ptr->value);
- if (!(ptr->o && ptr->option))
+ if (!(ptr->o && ptr->o->e.name))
return 0;
if ((ptr->o->type != UCI_TYPE_LIST))
@@ -669,7 +658,7 @@ int uci_del_list(struct uci_context *ctx, struct uci_ptr *ptr)
p = ptr->p;
if (!internal && p->has_delta)
- uci_add_delta(ctx, &p->delta, UCI_CMD_LIST_DEL, ptr->section, ptr->option, ptr->value);
+ uci_add_delta(ctx, &p->delta, UCI_CMD_LIST_DEL, ptr->s->e.name, ptr->o->e.name, ptr->value);
uci_foreach_element_safe(&ptr->o->v.list, tmp, e) {
if (!strcmp(ptr->value, uci_to_option(e)->e.name)) {
@@ -712,7 +701,7 @@ int uci_set(struct uci_context *ctx, struct uci_ptr *ptr)
} else if (!ptr->s && ptr->section) { /* new section */
ptr->s = uci_alloc_section(ptr->p, ptr->value, ptr->section, NULL);
ptr->last = &ptr->s->e;
- } else if (ptr->o && ptr->option) { /* update option */
+ } else if (ptr->o && ptr->o->e.name) { /* update option */
if (ptr->o->type == UCI_TYPE_STRING && !strcmp(ptr->o->v.string, ptr->value))
return 0;
@@ -720,13 +709,13 @@ int uci_set(struct uci_context *ctx, struct uci_ptr *ptr)
strcpy(ptr->o->v.string, ptr->value);
} else {
struct uci_option *old = ptr->o;
- ptr->o = uci_alloc_option(ptr->s, ptr->option, ptr->value, &old->e.list);
+ ptr->o = uci_alloc_option(ptr->s, old->e.name, ptr->value, &old->e.list);
if (ptr->option == old->e.name)
ptr->option = ptr->o->e.name;
uci_free_option(old);
ptr->last = &ptr->o->e;
}
- } else if (ptr->s && ptr->section) { /* update section */
+ } else if (ptr->s && ptr->s->e.name) { /* update section */
if (!strcmp(ptr->s->type, ptr->value))
return 0;
@@ -747,7 +736,7 @@ int uci_set(struct uci_context *ctx, struct uci_ptr *ptr)
}
if (!internal && ptr->p->has_delta)
- uci_add_delta(ctx, &ptr->p->delta, UCI_CMD_CHANGE, ptr->section, ptr->option, ptr->value);
+ uci_add_delta(ctx, &ptr->p->delta, UCI_CMD_CHANGE, ptr->s->e.name, ptr->o ? ptr->o->e.name : NULL, ptr->value);
return 0;
}
Several functions that have a uci_ptr as argument expect if ptr->s / ptr->o are provided that strcmp(ptr->s->e.name, ptr->section) == 0 and strcmp(ptr->o->e.name, ptr->option) == 0. Normally this is ensured by a call to uci_lookup_ptr that precedes the function invocation and by uci_expand_ptr. However, when using the C api directly ptr->section and ptr->option are not guarantied to be correct which can lead to unexpected or inconsistent behaviour (breaking delta tracking). Fix this by using ptr->section and ptr->option exclusively on new section, new option and in the case of uci_revert. In other cases use ptr->s->e.name and ptr->o->e.name exclusively. Signed-off-by: Jan Venekamp <jan@venekamp.net> --- list.c | 61 ++++++++++++++++++++++++---------------------------------- 1 file changed, 25 insertions(+), 36 deletions(-)