remove internal usage of redundant uci_ptr.last master
authorJan Venekamp <jan@venekamp.net>
Fri, 14 Jul 2023 18:28:11 +0000 (20:28 +0200)
committerHauke Mehrtens <hauke@hauke-m.de>
Thu, 10 Aug 2023 17:43:45 +0000 (19:43 +0200)
In uci_lookup_ptr and uci_set the pointer uci_ptr ptr.last is set to
the element corresponding to the first of: ptr.o, ptr.s, ptr.p.

Thus, ptr.last is redundant and in case of uci_set is (and was) not
always consistently set.

In order to simplify the code this commit removes internal usage
of ptr.last, and remove setting it from uci_set (and from uci_add_list
that was never used anyway).

As it is part of the public C api ptr.last cannot be completely
removed though. A search on lxr.openwrt.org shows that it is used as
the output of uci_lookup_ptr in several packages.

So we leave setting ptr.last in uci_lookup_ptr intact.

Signed-off-by: Jan Venekamp <jan@venekamp.net>
cli.c
delta.c
list.c
lua/uci.c

diff --git a/cli.c b/cli.c
index b4c4f9574b4daf1948a3e4be703ee48cd75fd5e6..f169e429478b12a56333aa0494cffe64ea616318 100644 (file)
--- a/cli.c
+++ b/cli.c
@@ -314,7 +314,6 @@ static void uci_show_changes(struct uci_package *p)
 
 static int package_cmd(int cmd, char *tuple)
 {
-       struct uci_element *e = NULL;
        struct uci_ptr ptr;
        int ret = 1;
 
@@ -323,7 +322,6 @@ static int package_cmd(int cmd, char *tuple)
                return 1;
        }
 
-       e = ptr.last;
        switch(cmd) {
        case CMD_CHANGES:
                uci_show_changes(ptr.p);
@@ -349,20 +347,14 @@ static int package_cmd(int cmd, char *tuple)
                        cli_perror();
                        goto out;
                }
-               switch(e->type) {
-                       case UCI_TYPE_PACKAGE:
-                               uci_show_package(ptr.p);
-                               break;
-                       case UCI_TYPE_SECTION:
-                               uci_show_section(ptr.s);
-                               break;
-                       case UCI_TYPE_OPTION:
-                               uci_show_option(ptr.o, true);
-                               break;
-                       default:
-                               /* should not happen */
-                               goto out;
-               }
+               if (ptr.o)
+                       uci_show_option(ptr.o, true);
+               else if (ptr.s)
+                       uci_show_section(ptr.s);
+               else if (ptr.p)
+                       uci_show_package(ptr.p);
+               else
+                       goto out; /* should not happen */
                break;
        }
 
@@ -475,7 +467,6 @@ done:
 
 static int uci_do_section_cmd(int cmd, int argc, char **argv)
 {
-       struct uci_element *e;
        struct uci_ptr ptr;
        int ret = UCI_OK;
        int dummy;
@@ -493,7 +484,6 @@ static int uci_do_section_cmd(int cmd, int argc, char **argv)
            (cmd != CMD_RENAME) && (cmd != CMD_REORDER))
                return 1;
 
-       e = ptr.last;
        switch(cmd) {
        case CMD_GET:
                if (!(ptr.flags & UCI_LOOKUP_COMPLETE)) {
@@ -501,17 +491,10 @@ static int uci_do_section_cmd(int cmd, int argc, char **argv)
                        cli_perror();
                        return 1;
                }
-               switch(e->type) {
-               case UCI_TYPE_SECTION:
-                       printf("%s\n", ptr.s->type);
-                       break;
-               case UCI_TYPE_OPTION:
+               if (ptr.o)
                        uci_show_value(ptr.o, false);
-                       break;
-               default:
-                       break;
-               }
-               /* throw the value to stdout */
+               else if (ptr.s)
+                       printf("%s\n", ptr.s->type);
                break;
        case CMD_RENAME:
                ret = uci_rename(ctx, &ptr);
diff --git a/delta.c b/delta.c
index 85ec970fb2452e60c8335021578e21203301c53d..5437fc1c4794291ef9c0d492fd7ef50cb14ba3f5 100644 (file)
--- a/delta.c
+++ b/delta.c
@@ -213,7 +213,6 @@ error:
 
 static void uci_parse_delta_line(struct uci_context *ctx, struct uci_package *p)
 {
-       struct uci_element *e = NULL;
        struct uci_ptr ptr;
        int cmd;
 
@@ -244,11 +243,14 @@ static void uci_parse_delta_line(struct uci_context *ctx, struct uci_package *p)
                UCI_INTERNAL(uci_del_list, ctx, &ptr);
                break;
        case UCI_CMD_ADD:
+               UCI_INTERNAL(uci_set, ctx, &ptr);
+               if (!ptr.option && ptr.s)
+                       ptr.s->anonymous = true;
+               break;
        case UCI_CMD_CHANGE:
                UCI_INTERNAL(uci_set, ctx, &ptr);
-               e = ptr.last;
-               if (!ptr.option && e && (cmd == UCI_CMD_ADD))
-                       uci_to_section(e)->anonymous = true;
+               break;
+       default:
                break;
        }
        return;
diff --git a/list.c b/list.c
index 16402130ebf2d87751c3ab7466155e9e4bcebc60..304c9e1fd1b279181345e841e28430b485eb76d6 100644 (file)
--- a/list.c
+++ b/list.c
@@ -616,7 +616,6 @@ int uci_add_list(struct uci_context *ctx, struct uci_ptr *ptr)
                UCI_TRAP_SAVE(ctx, error);
                ptr->o = uci_alloc_list(ptr->s, ptr->option, NULL);
                UCI_TRAP_RESTORE(ctx);
-               ptr->last = &ptr->o->e;
        } else if (ptr->o->type == UCI_TYPE_STRING) {
                /* create new list and add old string value as item to list */
                struct uci_option *old = ptr->o;
@@ -630,7 +629,6 @@ int uci_add_list(struct uci_context *ctx, struct uci_ptr *ptr)
                if (ptr->option == old->e.name)
                        ptr->option = ptr->o->e.name;
                uci_free_option(old);
-               ptr->last = &ptr->o->e;
        }
 
        /* add new item to list */
@@ -708,10 +706,8 @@ int uci_set(struct uci_context *ctx, struct uci_ptr *ptr)
                return uci_delete(ctx, ptr);
        } else if (!ptr->o && ptr->option) { /* new option */
                ptr->o = uci_alloc_option(ptr->s, ptr->option, ptr->value, NULL);
-               ptr->last = &ptr->o->e;
        } 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 */
                if (ptr->o->type == UCI_TYPE_STRING && !strcmp(ptr->o->v.string, ptr->value))
                        return 0;
@@ -724,7 +720,6 @@ int uci_set(struct uci_context *ctx, struct uci_ptr *ptr)
                        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 */
                if (!strcmp(ptr->s->type, ptr->value))
@@ -740,7 +735,6 @@ int uci_set(struct uci_context *ctx, struct uci_ptr *ptr)
                                ptr->section = ptr->s->e.name;
                        uci_free_section(old);
                        ptr->s->package->n_section--;
-                       ptr->last = &ptr->s->e;
                }
        } else {
                UCI_THROW(ctx, UCI_ERR_INVAL);
index 196a25bcf08a26e77c739c29b8ec6c66caf11d06..78b5e1f43827ead80350b246bbf8cde153512007 100644 (file)
--- a/lua/uci.c
+++ b/lua/uci.c
@@ -374,19 +374,16 @@ static int
 uci_lua_get_any(lua_State *L, bool all)
 {
        struct uci_context *ctx;
-       struct uci_element *e = NULL;
        struct uci_ptr ptr;
        int offset = 0;
        int nret = 1;
        char *s = NULL;
-       int err = UCI_ERR_NOTFOUND;
 
        ctx = find_context(L, &offset);
 
        if (lookup_args(L, ctx, offset, &ptr, &s))
                goto error;
 
-       lookup_ptr(ctx, &ptr, NULL, true);
        if (!all && !ptr.s) {
                ctx->err = UCI_ERR_INVAL;
                goto error;
@@ -396,33 +393,24 @@ uci_lua_get_any(lua_State *L, bool all)
                goto error;
        }
 
-       err = UCI_OK;
-       e = ptr.last;
-       switch(e->type) {
-               case UCI_TYPE_PACKAGE:
-                       uci_push_package(L, ptr.p);
-                       break;
-               case UCI_TYPE_SECTION:
-                       if (all) {
-                               uci_push_section(L, ptr.s, -1);
-                       }
-                       else {
-                               lua_pushstring(L, ptr.s->type);
-                               lua_pushstring(L, ptr.s->e.name);
-                               nret++;
-                       }
-                       break;
-               case UCI_TYPE_OPTION:
-                       uci_push_option(L, ptr.o);
-                       break;
-               default:
-                       ctx->err = UCI_ERR_INVAL;
-                       goto error;
+       if (ptr.o) {
+               uci_push_option(L, ptr.o);
+       } else if (ptr.s) {
+               if (all) {
+                       uci_push_section(L, ptr.s, -1);
+               }
+               else {
+                       lua_pushstring(L, ptr.s->type);
+                       lua_pushstring(L, ptr.s->e.name);
+                       nret++;
+               }
+       } else {
+               uci_push_package(L, ptr.p);
        }
+
        if (s)
                free(s);
-       if (!err)
-               return nret;
+       return nret;
 
 error:
        if (s)