[PATCH uci 2/2] remove internal usage of redundant uci_ptr.last

[Jan Venekamp]

On 01/08/2023 22:27, Hauke Mehrtens wrote:
> On 7/14/23 20:28, Jan Venekamp wrote:
>> 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 at venekamp.net>
>> ---
>>   cli.c     | 39 +++++++++++----------------------------
>>   delta.c   | 10 ++++++----
>>   list.c    |  6 ------
>>   lua/uci.c | 42 +++++++++++++++---------------------------
>>   4 files changed, 32 insertions(+), 65 deletions(-)
>>
> ......
>> @@ -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;
>
> How do we make sure that only one of these is set at a time?
> Before the code would print the last element added.
>

The code in uci_lookup_ptr makes sure that the element being looked up 
is the last of: ptr.p, ptr.s, ptr.o with a non null value (pointer).

Thus, if an option is looked up ptr.o is set and printed. When a section 
is looked up ptr.o is null, but ptr.s is set and printed. Else a package 
is looked up, ptr.o and ptr.s are null so ptr.p is printed. (When a 
looked up element is not found UCI_LOOKUP_COMPLETE is not set which will 
result in an error.)

So the code prints the same element as before.

Hope this explains it.

>>       }
>>   @@ -475,7 +467,6 @@ done:
> ......