Re: [PATCH uci] file: Fix uci -m import command

[Jan Venekamp]

On 13/07/2023 18:53, Hauke Mehrtens wrote:

Without this change we see the following error:
   # uci -m import optic < /etc/optic-db/default
   uci: Parse error (option/list command found before the first section) at 
line 4, byte 1

ptr.last is still a null pointer in case the uci_lookup_list() call
found a matching section and set ptr.s to it. The code expects that
uci_set() updates the ptr.last pointer, but this is not done any more


Ah snap, I overlooked that in uci_parse_config while working on uci_set.

A clearer fix might be to just change the last line of uci_parse_config to:
pctx->section = ptr.s;
The change to uci_parse_option does not do anything, ptr->last is never 
read after being set.


However, seeing this made me more convinced that ptr.last just 
complicates the code while being redundant anyway. So I created a patch 
that removes internal usage of ptr.last altogether. Would you please be 
so kind to take a look at it?

https://patchwork.ozlabs.org/project/openwrt/list/?series=363985

While your at it, maybe you could also take a look at these other 
patches for uci?

https://patchwork.ozlabs.org/project/openwrt/patch/20221120010927.23856-2-...@venekamp.net/
https://patchwork.ozlabs.org/project/openwrt/patch/sy4p282mb39395204ae37bc45d35d834cc5...@sy4p282mb3939.ausp282.prod.outlook.com/

Kind regards,
Jan Venekamp


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

[PATCH uci] file: Fix uci -m import command

[Hauke Mehrtens via openwrt-devel]

The sender domain has a DMARC Reject/Quarantine policy which disallows
sending mailing list messages using the original "From" header.

To mitigate this problem, the original message has been wrapped
automatically by the mailing list software.--- Begin Message ---
Without this change we see the following error:
  # uci -m import optic < /etc/optic-db/default
  uci: Parse error (option/list command found before the first section) at line 
4, byte 1

ptr.last is still a null pointer in case the uci_lookup_list() call
found a matching section and set ptr.s to it. The code expects that
uci_set() updates the ptr.last pointer, but this is not done any more.
If case uci_lookup_list() did not found a section ptr->s is a null
pointer and then uci_set() will allocate a new section.

Fixes: ae61e1cad4a1 ("uci: optimize update section in uci_set")
Fixes: 7e01d66d7bec ("uci: optimize update option in uci_set")
Signed-off-by: Hauke Mehrtens 
---
 file.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/file.c b/file.c
index 93abfae..b01480c 100644
--- a/file.c
+++ b/file.c
@@ -449,6 +449,7 @@ static void uci_parse_config(struct uci_context *ctx)
e = uci_lookup_list(>package->sections, name);
if (e) {
ptr.s = uci_to_section(e);
+   ptr.last = >e;
 
if ((ctx->flags & UCI_FLAG_STRICT) && 
strcmp(ptr.s->type, type))
uci_parse_error(ctx, "section of different type 
overwrites prior section with same name");
@@ -490,8 +491,10 @@ static void uci_parse_option(struct uci_context *ctx, bool 
list)
 
uci_fill_ptr(ctx, , >section->e);
e = uci_lookup_list(>section->options, name);
-   if (e)
+   if (e) {
ptr.o = uci_to_option(e);
+   ptr.last = >e;
+   }
ptr.option = name;
ptr.value = value;
 
-- 
2.17.1


--- End Message ---
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel