[PATCH 1/1] uci: ignore wrong section / option name in uci_ptr

Jan Venekamp jan at venekamp.net
Sat Nov 19 17:09:27 PST 2022


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 at venekamp.net>
---
 list.c | 61 ++++++++++++++++++++++++----------------------------------
 1 file changed, 25 insertions(+), 36 deletions(-)

diff --git a/list.c b/list.c
index 1640213..87b02ad 100644
--- a/list.c
+++ b/list.c
@@ -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;
 }
-- 
2.32.0 (Apple Git-132)




More information about the openwrt-devel mailing list