[PATCH] grubenv module rewritten

[Maciej Pijanowski]

---
Hello Stefano,

I'm sending rewritten grubenv module. Please note that no significant changes
in swupdate source has been made since last patch. You can skip those at the
moment. I'll try to address your previous remarks in an upcoming time.

Written module may seem to have quite a lot of functions. I wanted to split it into
parts so I could test each one separately during development. We can refactor
if you find it necessary. You can check out test I came up with during
development [1]. I'm not sure what's the approach of this project in terms of
validation. Is it desired to include some unit tests for modules? If so what
would be the appropriate form (framework).

[1] https://github.com/3mdeb/grubenv

Regards,
Maciej Pijanowski

Makefile | 13 +-
bootloader/Makefile | 1 +
bootloader/grubenv.c | 315 ++++++++++++++++++++++++++++++++++++++++++++++
corelib/installer.c | 34 +++++
examples/config/defconfig | 82 ++++++++++++
handlers/Config.in | 14 +++
include/grubenv.h | 44 +++++++
include/swupdate.h | 1 +
parser/parser.c | 50 ++++++++
9 files changed, 553 insertions(+), 1 deletion(-)
create mode 100644 bootloader/Makefile
create mode 100644 bootloader/grubenv.c
create mode 100644 examples/config/defconfig
create mode 100644 include/grubenv.h

diff --git a/Makefile b/Makefile
index c6f8389969a2..c101c70933ad 100644
--- a/Makefile
+++ b/Makefile
@@ -4,6 +4,10 @@ SUBLEVEL = 0
EXTRAVERSION = -rc1
NAME =

+INSTALL = install
+PREFIX ?= /usr/local
+BINDIR ?= $(PREFIX)/bin
+
# *DOCUMENTATION*
# To see a list of typical targets execute "make help"
# More info can be located in ./README
@@ -344,7 +348,7 @@ include $(srctree)/Makefile.flags
all: swupdate progress

objs-y := core handlers
-libs-y := archival corelib ipc mongoose parser suricatta
+libs-y := archival corelib ipc mongoose parser suricatta bootloader
client-y := progress_client

swupdate-dirs := $(objs-y) $(libs-y)
@@ -517,6 +521,13 @@ endif # skip-makefile
PHONY += FORCE
FORCE:

+# install
+#
+PHONY += install
+install:
+ $(INSTALL) -d ${BINDIR}/
+ $(INSTALL) -m 0755 swupdate ${BINDIR}/
+
# Documentation
# run Makefile in doc directory

diff --git a/bootloader/Makefile b/bootloader/Makefile
new file mode 100644
index 000000000000..f115ee98647a
--- /dev/null
+++ b/bootloader/Makefile
@@ -0,0 +1 @@
+lib-$(CONFIG_GRUB) += grubenv.o
diff --git a/bootloader/grubenv.c b/bootloader/grubenv.c
new file mode 100644
index 000000000000..f78c1b0aa811
--- /dev/null
+++ b/bootloader/grubenv.c
@@ -0,0 +1,315 @@
+/*
+ * Author: Maciej Pijanowski maciej.p...@3mdeb.com
+ * Copyright (C) 2017, 3mdeb
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include "util.h"
+#include "grubenv.h"
+
+
+/* read environment from storage into RAM */
+char *grubenv_open(const char *grubenv_file)
+{
+ char *grubenv;
+ FILE *fp;
+ size_t size;
+
+ fp = fopen(grubenv_file, "rb");
+ if (!fp) {
+ ERROR("Failed to open grubenv file: %s\n", grubenv_file);
+ return NULL;
+ }
+
+ if (fseek(fp, 0, SEEK_END)) {
+ ERROR("Failed to seek end grubenv file: %s\n", grubenv_file);
+ return NULL;
+ }
+
+ size = (size_t)ftell(fp);
+
+ if (size != GRUBENV_SIZE) {
+ ERROR("Ivalid grubenv file size: %d\n", (int)size);
+ return NULL;
+ }
+
+ if (fseek(fp, 0, SEEK_SET)) {
+ ERROR("Failed to seek set grubenv file: %s\n", grubenv_file);
+ return NULL;
+ }
+
+ grubenv = malloc(size);
+ if (!grubenv) {
+ ERROR("Not enough memory for environment\n");
+ return NULL;
+ }
+
+ if (fread(grubenv, 1, size, fp) != size) {
+ ERROR("Failed to read %s\n", grubenv);
+ return NULL;
+ }
+
+ fclose(fp);
+
+ if (memcmp(grubenv, GRUBENV_HEADER, sizeof(GRUBENV_HEADER) -1)) {
+ ERROR("Invalid grubenv header\n");
+ return NULL;
+ }
+
+ return grubenv;
+}
+
+char *grubenv_find(const char *grubenv, const char *name)
+{
+ char *ptrline, *string;
+
+ /* form string "name=" */
+ /* space for '=' and '\0' */
+ string = malloc(strlen(name) + 2);
+ if (!string) {
+ ERROR("Not enough memory\n");
+ return NULL;
+ }
+ /* copy with '\0' */
+ strncpy(string, name, strlen(name) + 1);
+ /* '\0' is always appended */
+ strncat(string, "=", 1);
+ ptrline = strstr(grubenv, string);
+ free(string);
+
+ /* return pointer to the beginning of found line */
+ return ptrline;
+}
+
+int grubenv_llen(char *ptrline)
+{
+ char *ptrstart = ptrline;
+ while (*ptrline != '\n') {
+ ptrline++;
+ }
+ return ptrline - ptrstart + 1;
+}
+
+void grubenv_remove(char *grubenv, char *ptrline, int space)
+{
+ int llen = grubenv_llen(ptrline);
+ char *ptrend = grubenv + GRUBENV_SIZE - 1;
+
+ /* copy block after given line at the beginning of this line */
+ memmove(ptrline, ptrline + llen, ptrend - ptrline - llen);
+ /* fill space after last variable with # */
+ memset(ptrend - space - llen + 1, '#', space + llen);
+}
+
+int grubenv_space(char *grubenv)
+{
+ char *ptr, *ptrend;
+ int space;
+
+ /* pointer to the last character */
+ ptrend = grubenv + GRUBENV_SIZE - 1;
+ ptr = ptrend;
+
+ /* empty space in bytes - actually it's number of # characters */
+ space = 0;
+ while(*ptr == '#') {
+ ptr--;
+ space++;
+ }
+
+ return space;
+}
+
+/* append single variable at the end of envblock stored in RAM */
+int grubenv_append(char *grubenv, const char *name, const char *value,
+ int space)
+{
+ int llen;
+ char *ptr, *line;
+
+ if (!name) {
+ ERROR("Invalid variable name: %s\n", name);
+ return -1;
+ }
+
+ /* 2 additional bytes: '=' and '\n' */
+ llen = strlen(name) + strlen(value) + 2;
+
+ /* ptr points now at first non '#' character */
+ ptr = grubenv + GRUBENV_SIZE - 1 - space;
+
+ /* it should be '\n' */
+ if (*ptr != '\n') {
+ ERROR("Invalid environment block\n");
+ return -1;
+ }
+
+ /* one byte forward - beginning of a new line */
+ ptr++;
+
+ /* formatted name=value string */
+ line = malloc(llen);
+ if (!line) {
+ ERROR("Not enough memory\n");
+ return -ENOMEM;
+ }
+ snprintf(line, llen+1, "%s=%s\n", name, value);
+ memcpy(ptr, line, llen);
+ free(line);
+
+ return 0;
+}
+
+int grubenv_write(const char *grubenv_file, const char *grubenv)
+{
+ FILE *fp;
+ char *grubenv_file_new;
+ int ret;
+
+ /* 5 bytes for '.new' and '\0' */
+ grubenv_file_new = malloc(strlen(grubenv_file) + 5);
+ if (!grubenv_file_new) {
+ ERROR("Not enough memory\n");
+ return -ENOMEM;
+ }
+ /* copy with '\0' */
+ strncpy(grubenv_file_new, grubenv_file, strlen(grubenv_file) + 1);
+ /* copy '.new' string, '\0' is always appended */
+ strncat(grubenv_file_new, ".new", 4);
+
+ fp = fopen(grubenv_file_new, "wb");
+ if (!fp) {
+ ERROR("Failed to open file: %s", grubenv_file_new);
+ return -1;
+ }
+
+ ret = fwrite(grubenv, 1, GRUBENV_SIZE, fp);
+ if (ret != GRUBENV_SIZE) {
+ ERROR("Failed to write file: %s. Bytes written: %d\n",
+ grubenv_file_new, ret);
+ return -1;
+ }
+
+ fclose(fp);
+
+ if (rename(grubenv_file_new, grubenv_file)) {
+ ERROR("Failed to move environment: %s into %s\n",
+ grubenv_file_new, grubenv_file);
+ return -1;
+ }
+
+ free(grubenv_file_new);
+ return 0;
+}
+
+int grubenv_set(const char *grubenv_file, const char *name, const char *value)
+{
+ char *grubenv;
+ char *found;
+ int llen, space, ret;
+
+ grubenv = grubenv_open(grubenv_file);
+ if (!grubenv) {
+ ret = -1;
+ goto fail;
+ }
+ space = grubenv_space(grubenv);
+
+ found = grubenv_find(grubenv, name);
+ /* variable already exists in environment */
+ if (found) {
+ /* length of line to be removed */
+ llen = grubenv_llen(found);
+ /* just remove line with found variable */
+ grubenv_remove(grubenv, found, space);
+ space += llen;
+ }
+ /* length of line to be added */
+ /* 2 additional bytes: '=' and '\n' */
+ llen = strlen(name) + strlen(value) + 2;
+
+ if (llen < space){
+ /* append new variable at the end */
+ ret = grubenv_append(grubenv, name, value, space);
+ if (ret)
+ goto fail;
+ ret = grubenv_write(grubenv_file, grubenv);
+ if (ret)
+ goto fail;
+ }
+ else {
+ ret = -1;
+ ERROR("Not enough free space in envblk file. Available: %d \
+ , required %d", space, llen);
+ goto fail;
+ }
+
+ grubenv_close(grubenv);
+ return 0;
+
+ fail:
+ grubenv_close(grubenv);
+ return ret;
+}
+
+int grubenv_unset(const char *grubenv_file, const char *name)
+{
+ char *grubenv;
+ char *found;
+ int space, ret;
+
+ grubenv = grubenv_open(grubenv_file);
+ if (!grubenv) {
+ ret = -1;
+ goto fail;
+ }
+
+ found = grubenv_find(grubenv, name);
+ /* variable already exists in environment */
+ if (found) {
+ /* how many '#' characters to set after var removal */
+ space = grubenv_space(grubenv);
+ space += grubenv_llen(found);
+ /* remove line with found variable */
+ grubenv_remove(grubenv, found, space);
+ ret = grubenv_write(grubenv_file, grubenv);
+ if (ret)
+ goto fail;
+ }
+ /*
+ else {
+ log if variable not found ?
+ }
+ */
+
+ grubenv_close(grubenv);
+ return 0;
+
+ fail:
+ grubenv_close(grubenv);
+ return ret;
+}
+
+/* free environment data from RAM */
+/* ATM it's just for completeness */
+void grubenv_close(char *grubenv)
+{
+ free(grubenv);
+}
diff --git a/corelib/installer.c b/corelib/installer.c
index fc5afb473758..e9c08b5be103 100644
--- a/corelib/installer.c
+++ b/corelib/installer.c
@@ -44,6 +44,10 @@
#include "fw_env.h"
#include "progress.h"

+#ifdef CONFIG_GRUB
+#include "grubenv.h"
+#endif /* CONFIG_GRUB */
+
static int isImageInstalled(struct swver *sw_ver_list,
struct img_type *img)
{
@@ -208,6 +212,28 @@ static int update_uboot_env(void)
return ret;
}

+#ifdef CONFIG_GRUB
+static int update_grub_env(struct swupdate_cfg *cfg)
+{
+ int ret = 0;
+
+ struct dict_entry *grubvar;
+ TRACE("Updating GRUB environment");
+
+ LIST_FOREACH(grubvar, &cfg->grub, next) {
+ if (!grubvar->varname || !grubvar->value)
+ continue;
+ ret = grubenv_set(GRUBENV_PATH, grubvar->varname,
+ grubvar->value);
+ if (ret < 0) {
+ ERROR("Error updating GRUB environment");
+ break;
+ }
+ }
+ return ret;
+}
+#endif /* CONFIG_GRUB */
+
int install_single_image(struct img_type *img)
{
struct installer_handler *hnd;
@@ -270,6 +296,14 @@ int install_images(struct swupdate_cfg *sw, int fdsw, int fromfile)
return ret;
}

+#ifdef CONFIG_GRUB
+ /* Update GRUB environment */
+ ret = update_grub_env(sw);
+ if (ret) {
+ return ret;
+ }
+#endif /* CONFIG_GRUB */
+
LIST_FOREACH(img, &sw->images, next) {

/*
diff --git a/examples/config/defconfig b/examples/config/defconfig
new file mode 100644
index 000000000000..a330c8a3f9dd
--- /dev/null
+++ b/examples/config/defconfig
@@ -0,0 +1,82 @@
+#
+# Automatically generated file; DO NOT EDIT.
+# Swupdate Configuration
+#
+CONFIG_HAVE_DOT_CONFIG=y
+
+#
+# Swupdate Settings
+#
+
+#
+# General Configuration
+#
+CONFIG_SCRIPTS=y
+# CONFIG_HW_COMPATIBILITY is not set
+CONFIG_SW_VERSIONS_FILE="/etc/sw-versions"
+# CONFIG_MTD is not set
+CONFIG_LUA=y
+CONFIG_LUAPKG="lua50"
+# CONFIG_FEATURE_SYSLOG is not set
+
+#
+# Build Options
+#
+# CONFIG_STATIC is not set
+CONFIG_CROSS_COMPILE=""
+CONFIG_SYSROOT=""
+CONFIG_EXTRA_CFLAGS=""
+CONFIG_EXTRA_LDFLAGS=""
+CONFIG_EXTRA_LDLIBS=""
+
+#
+# Debugging Options
+#
+# CONFIG_DEBUG is not set
+# CONFIG_WERROR is not set
+# CONFIG_NOCLEANUP is not set
+CONFIG_DOWNLOAD=y
+# CONFIG_HASH_VERIFY is not set
+# CONFIG_SIGNED_IMAGES is not set
+# CONFIG_ENCRYPTED_IMAGES is not set
+# CONFIG_SURICATTA is not set
+
+#
+# Suricatta
+#
+
+#
+# Server
+#
+CONFIG_SURICATTA_SERVER_NONE=y
+
+#
+# hawkBit support needs libcurl and CONFIG_JSON=y
+#
+# CONFIG_WEBSERVER is not set
+
+#
+# Archival Features
+#
+CONFIG_GUNZIP=y
+
+#
+# Parser Features
+#
+CONFIG_LIBCONFIG=y
+CONFIG_LIBCONFIGROOT=""
+# CONFIG_JSON is not set
+# CONFIG_LUAEXTERNAL is not set
+# CONFIG_SETSWDESCRIPTION is not set
+
+#
+# Image Handlers
+#
+CONFIG_RAW=y
+# CONFIG_LUASCRIPTHANDLER is not set
+CONFIG_SHELLSCRIPTHANDLER=y
+CONFIG_ARCHIVE=y
+# CONFIG_REMOTE_HANDLER is not set
+# CONFIG_UBOOT is not set
+CONFIG_GRUB=y
+CONFIG_GRUB_ENV="/boot/efi/EFI/BOOT/grub/grubenv"
diff --git a/handlers/Config.in b/handlers/Config.in
index a05c1fc1ef9a..e3ad342a4bb9 100644
--- a/handlers/Config.in
+++ b/handlers/Config.in
@@ -148,4 +148,18 @@ config UBOOT_FWENV
in the tools directory. It tells where the U-Boot
environment is saved.

+config GRUB
+ bool "grub"
+ default n
+ help
+ Enable it to change GRUB environment
+ during the installation process.
+
+config GRUBENV
+ string "GRUB Environment block file path"
+ depends on GRUB
+ default "/boot/efi/EFI/BOOT/grub/grubenv"
+ help
+ Provide path to grub environment block file
+
endmenu
diff --git a/include/grubenv.h b/include/grubenv.h
new file mode 100644
index 000000000000..8b722e2a2839
--- /dev/null
+++ b/include/grubenv.h
@@ -0,0 +1,44 @@
+/*
+ * Author: Maciej Pijanowski, maciej.p...@3mdeb.com
+ * Copyright (C) 2017, 3mdeb
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc.
+ */
+
+#define GRUBENV_SIZE 1024 /* bytes */
+#define GRUBENV_HEADER "# GRUB Environment Block\n"
+#define GRUBENV_DEFAULT_PATH "/boot/efi/EFI/BOOT/grub/grubenv"
+
+#ifdef CONFIG_GRUBENV
+#define GRUBENV_PATH CONFIG_GRUBENV
+#else
+#define GRUBENV_PATH GRUBENV_DEFAULT_PATH
+#endif
+
+/* Internal functions */
+/* Exposed here for unit tests during development */
+char *grubenv_open(const char *grubenv_file);
+char *grubenv_find(const char *grubenv, const char *name);
+void grubenv_remove(char *grubenv, char *ptrline, int space);
+void grubenv_close(char *grubenv);
+int grubenv_append(char *grubenv, const char *name, const char *value,
+ int space);
+int grubenv_space(char *grubenv);
+int grubenv_write(const char *grubenv_file, const char *grubenv);
+int grubenv_llen(char *ptrline);
+
+/* only these should be called from external */
+int grubenv_set(const char *grubenv_file, const char *name, const char *value);
+int grubenv_unset(const char *grubenv_file, const char *name);
diff --git a/include/swupdate.h b/include/swupdate.h
index bff757ebff7a..b21f4793ee78 100644
--- a/include/swupdate.h
+++ b/include/swupdate.h
@@ -123,6 +123,7 @@ struct swupdate_cfg {
struct imglist partitions;
struct imglist scripts;
struct dictlist uboot;
+ struct dictlist grub;
void *dgst; /* Structure for signed images */
struct swupdate_global_cfg globals;
};
diff --git a/parser/parser.c b/parser/parser.c
index 33a496085441..4e635b8cccc3 100644
--- a/parser/parser.c
+++ b/parser/parser.c
@@ -349,6 +349,50 @@ static void parse_uboot(parsertype p, void *cfg, struct swupdate_cfg *swcfg)
}
}

+#ifdef CONFIG_GRUB
+static void parse_grub(parsertype p, void *cfg, struct swupdate_cfg *swcfg)
+{
+ void *setting, *elem;
+ int count, i;
+ char name[32];
+ char value[255];
+
+ setting = find_node(p, cfg, "grub", swcfg);
+
+ if (setting == NULL)
+ return;
+
+ count = get_array_length(p, setting);
+ for(i = (count - 1); i >= 0; --i) {
+ elem = get_elem_from_idx(p, setting, i);
+
+ if (!elem)
+ continue;
+
+ /*
+ * Check for mandatory field
+ */
+ if(!(exist_field_string(p, elem, "name"))) {
+ TRACE("GRUB entry without variable name field, skipping..");
+ continue;
+ }
+
+ /*
+ * Call directly get_field_string with size 0
+ * to let allocate the place for the strings
+ */
+ GET_FIELD_STRING(p, elem, "name", name);
+ GET_FIELD_STRING(p, elem, "value", value);
+ dict_set_value(&swcfg->grub, name, value);
+
+ TRACE("GRUB var: %s = %s\n",
+ name,
+ dict_get_value(&swcfg->grub, name));
+
+ }
+}
+#endif /* config_grub */
+
static void parse_images(parsertype p, void *cfg, struct swupdate_cfg *swcfg)
{
void *setting, *elem;
@@ -496,6 +540,9 @@ static int parser(parsertype p, void *cfg, struct swupdate_cfg *swcfg)
parse_images(p, cfg, swcfg);
parse_scripts(p, cfg, swcfg);
parse_uboot(p, cfg, swcfg);
+#ifdef CONFIG_GRUB
+ parse_grub(p, cfg, swcfg);
+#endif /* config_grub */
parse_files(p, cfg, swcfg);

/*
@@ -507,6 +554,9 @@ static int parser(parsertype p, void *cfg, struct swupdate_cfg *swcfg)
if (LIST_EMPTY(&swcfg->images) &&
LIST_EMPTY(&swcfg->partitions) &&
LIST_EMPTY(&swcfg->scripts) &&
+#ifdef CONFIG_GRUB
+ LIST_EMPTY(&swcfg->grub) &&
+#endif /* config_grub */
LIST_EMPTY(&swcfg->uboot)) {
ERROR("Found nothing to install\n");
return -1;
--
2.7.4

[Stefano Babic]

Hi Maciej,

On 30/03/2017 10:32, Maciej Pijanowski wrote:
> ---
> Hello Stefano,
>
> I'm sending rewritten grubenv module. Please note that no significant changes
> in swupdate source has been made since last patch.

ok, do not worry.

> You can skip those at the
> moment. I'll try to address your previous remarks in an upcoming time.

ok

>
> Written module may seem to have quite a lot of functions. I wanted to split it into
> parts so I could test each one separately during development. We can refactor
> if you find it necessary. You can check out test I came up with during
> development [1]. I'm not sure what's the approach of this project in terms of
> validation. Is it desired to include some unit tests for modules?

It is not mandatory. Of course, I have nothing against them.

Hawkbit's module has a unit test based on cmoka. I am not so happy
because it constraints to change code, that means to declare functions
global instead of static as they should be.

I am currently thinking about test environment to run tests on real
hardware instead of just test unit.

This has nothing to do, I suppose.

> # *DOCUMENTATION*
> # To see a list of typical targets execute "make help"
> # More info can be located in ./README
> @@ -344,7 +348,7 @@ include $(srctree)/Makefile.flags
> all: swupdate progress
>
> objs-y := core handlers
> -libs-y := archival corelib ipc mongoose parser suricatta
> +libs-y := archival corelib ipc mongoose parser suricatta bootloader
> client-y := progress_client
>

I think we should agree about the interface we want to expose. My idea
is that we have a CONFIG_BOOTLOADER in Kconfig, and we can select which
bootloader in a option menu (U-Boot or Grub).

THe interface to the bootloader should be something as:

- read an environment variabel (now fw_env_get(), ==> bootloader_get_env)

- write an emvironment variable ==> bootloader_env_set()

- apply a set of variables to make an atomic switch ==>
bootloader_apply_list() (in U-Boot this is now fw_parse_script)

Grub and U-Boot should expose the same functions, but just one of them
can be activated. The rest of code is not aware which is the bootloader,
it is enough to call one function of this API.

> swupdate-dirs := $(objs-y) $(libs-y)
> @@ -517,6 +521,13 @@ endif # skip-makefile
> PHONY += FORCE
> FORCE:
>
> +# install
> +#
> +PHONY += install

Why ? install is generally not in PHONY and must be explicitely called.
If put here, a make install is even called when building in Yocto, and
this is not desired .

So I am expecting it does not return a generic string, but the pointer
to the 1024 byte block. It should be better to define a type for it.

I can guess that this function as most of functions here can be declared
static.

> +void grubenv_remove(char *grubenv, char *ptrline, int space)
> +{
> + int llen = grubenv_llen(ptrline);
> + char *ptrend = grubenv + GRUBENV_SIZE - 1;
> +
> + /* copy block after given line at the beginning of this line */
> + memmove(ptrline, ptrline + llen, ptrend - ptrline - llen);
> + /* fill space after last variable with # */
> + memset(ptrend - space - llen + 1, '#', space + llen);
> +}

I am just thinking about if there are another easier way to do this. You
work with environment in memory. That means you are not constrained to
have the same format (the 1024 bytes block) as the storage. You could
read the grubenv file, use your own format, and writing then the 1024
block in the write.

I say this because the API is then too specific to Grub, or better, to
the grub format. ptrline and space have sense just thinking to the grub
block, but it is then difficult to generalize. As I said, I need to find
the same interface for all bootloaders.

What about to reuse the functions I have written in swupdate_dict.c ? It
looks like that code can be easier:

- you open the grubenv ==> in your open, you decode the 1024 block
calling dict_set_value() for each variable you find.

- dropping / changing values becomes then easy because it requires just
to traverse the list

- in the write, you need to "serialize" the list, that means you iterate
the list writing into the 1024 block. But you do not need to search for
positions inside the block, and even dropping a variable is just
removing the entry instead of reassembling the whole block.

Waht do you think ?

With the list, you do not need append / insert/...

Of course - we need the same interface for all bootloaders.And of
course, I will change on my side the U-Boot's interface to made it
generic - so do not stick with current U-Boot's interface, we change it
if needed.

Best regards,
Stefano

--
=====================================================================
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=====================================================================