[PATCH 2/5] uservars: change type data field from char* to uint64_t

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

Store user variable types as uint64_t instead of a string. This enables
the possibility of definint flags. Flags are used to delete a user
variable (to destinguish between setting it empty or removing it) and to
mark variables with special properties. This will be used in a following
commit to define global user variables.

Bits 0..31 are reserved for standard datatype definitions,
bits 32..48 are reserved for user defined datatypes or flags,
bits 49..63 are reserved for internal flags.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
env/env_api.c | 9 ++++----
env/env_api_fat.c | 16 +++++++------
env/uservars.c | 42 +++++++++++++++++-----------------
include/ebgenv.h | 24 +++++++++++++++----
include/env_api.h | 8 +++----
include/uservars.h | 13 ++++++-----
tools/bg_setenv.c | 38 +++++++++++++-----------------
tools/tests/test_ebgenv_api.c | 33 ++++++++++++++------------
tools/tests/test_ebgenv_api_internal.c | 34 +++++++++++++--------------
9 files changed, 114 insertions(+), 103 deletions(-)

diff --git a/env/env_api.c b/env/env_api.c
index 76f7b93..bd034af 100644
--- a/env/env_api.c
+++ b/env/env_api.c
@@ -99,7 +99,7 @@ int ebg_env_get(ebgenv_t *e, char *key, char *buffer)
ENV_STRING_LENGTH);
}

-int ebg_env_get_ex(ebgenv_t *e, char *key, char *usertype, uint8_t *buffer,
+int ebg_env_get_ex(ebgenv_t *e, char *key, uint64_t *usertype, uint8_t *buffer,
uint32_t maxlen)
{
return bgenv_get((BGENV *)e->bgenv, key, usertype, buffer, maxlen);
@@ -111,7 +111,7 @@ int ebg_env_set(ebgenv_t *e, char *key, char *value)
strlen(value) + 1);
}

-int ebg_env_set_ex(ebgenv_t *e, char *key, char *usertype, uint8_t *value,
+int ebg_env_set_ex(ebgenv_t *e, char *key, uint64_t usertype, uint8_t *value,
uint32_t datalen)
{
return bgenv_set((BGENV *)e->bgenv, key, usertype, value, datalen);
@@ -174,9 +174,8 @@ int ebg_env_setglobalstate(ebgenv_t *e, uint16_t ustate)
return EINVAL;
}
(void)snprintf(buffer, sizeof(buffer), "%d", ustate);
- res =
- bgenv_set((BGENV *)e->bgenv, "ustate", "String", buffer,
- strlen(buffer) + 1);
+ res = bgenv_set((BGENV *)e->bgenv, "ustate", 0, buffer,
+ strlen(buffer) + 1);

if (ustate != USTATE_OK) {
return res;
diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 8eb1454..7d5e714 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c
@@ -236,7 +236,8 @@ bool bgenv_close(BGENV *env)
return false;
}

-int bgenv_get(BGENV *env, char *key, char *type, void *data, uint32_t maxlen)
+int bgenv_get(BGENV *env, char *key, uint64_t *type, void *data,
+ uint32_t maxlen)
{
EBGENVKEY e;
char buffer[ENV_STRING_LENGTH];
@@ -267,7 +268,7 @@ int bgenv_get(BGENV *env, char *key, char *type, void *data, uint32_t maxlen)
}
strncpy(data, buffer, strlen(buffer)+1);
if (type) {
- sprintf(type, "char*");
+ *type = USERVAR_TYPE_STRING_ASCII;
}
break;
case EBGENV_KERNELPARAMS:
@@ -277,7 +278,7 @@ int bgenv_get(BGENV *env, char *key, char *type, void *data, uint32_t maxlen)
}
strncpy(data, buffer, strlen(buffer)+1);
if (type) {
- sprintf(type, "char*");
+ *type = USERVAR_TYPE_STRING_ASCII;
}
break;
case EBGENV_WATCHDOG_TIMEOUT_SEC:
@@ -287,7 +288,7 @@ int bgenv_get(BGENV *env, char *key, char *type, void *data, uint32_t maxlen)
}
strncpy(data, buffer, strlen(buffer)+1);
if (type) {
- sprintf(type, "uint16_t");
+ *type = USERVAR_TYPE_UINT16;
}
break;
case EBGENV_REVISION:
@@ -297,7 +298,7 @@ int bgenv_get(BGENV *env, char *key, char *type, void *data, uint32_t maxlen)
}
strncpy(data, buffer, strlen(buffer)+1);
if (type) {
- sprintf(type, "uint32_t");
+ *type = USERVAR_TYPE_UINT16;
}
break;
case EBGENV_USTATE:
@@ -307,7 +308,7 @@ int bgenv_get(BGENV *env, char *key, char *type, void *data, uint32_t maxlen)
}
strncpy(data, buffer, strlen(buffer)+1);
if (type) {
- sprintf(type, "uint16_t");
+ *type = USERVAR_TYPE_UINT16;
}
break;
default:
@@ -319,7 +320,8 @@ int bgenv_get(BGENV *env, char *key, char *type, void *data, uint32_t maxlen)
return 0;
}

-int bgenv_set(BGENV *env, char *key, char *type, void *data, uint32_t datalen)
+int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
+ uint32_t datalen)
{
EBGENVKEY e;
int val;
diff --git a/env/uservars.c b/env/uservars.c
index cd3f24b..f6013c1 100644
--- a/env/uservars.c
+++ b/env/uservars.c
@@ -14,21 +14,21 @@
#include "env_api.h"
#include "uservars.h"

-void bgenv_map_uservar(uint8_t *udata, char **key, char **type, uint8_t **val,
+void bgenv_map_uservar(uint8_t *udata, char **key, uint64_t *type, uint8_t **val,
uint32_t *record_size, uint32_t *data_size)
{
/* Each user variable is encoded as follows:
- * |------------|--------------|-------------|----------------|
- * | char KEY[] | uint32_t len | char type[] | uint8_t data[] |
- * |------------|--------------|-------------|----------------|
- * | KEY | < - - - - - - - - PAYLOAD - - - - - - - - > |
+ * |------------|--------------|---------------|----------------|
+ * | char KEY[] | uint32_t len | uint64_t type | uint8_t data[] |
+ * |------------|--------------|---------------|----------------|
+ * | KEY | < - - - - - - - - PAYLOAD - - - - - - - - - > |
*
* here char[] is a null-terminated string
* 'len' is the payload size (visualized by the horizontal dashes)
*/
char *var_key;
uint32_t *payload_size;
- char *var_type;
+ uint64_t *var_type;
uint8_t *data;

/* Get the key */
@@ -46,24 +46,24 @@ void bgenv_map_uservar(uint8_t *udata, char **key, char **type, uint8_t **val,
}

/* Get position of the type field */
- var_type = (char *)payload_size + sizeof(uint32_t);
+ var_type = (uint64_t *)((uint8_t *)payload_size + sizeof(uint32_t));
if (type) {
- *type = var_type;
+ *type = *var_type;
}

/* Calculate the data size */
if (data_size) {
*data_size = *payload_size - sizeof(uint32_t) -
- strlen(var_type) - 1;
+ sizeof(uint64_t);
}
/* Get the pointer to the data field */
- data = (uint8_t *)(var_type + strlen(var_type) + 1);
+ data = (uint8_t *)(var_type + sizeof(uint64_t));
if (val) {
*val = data;
}
}

-void bgenv_serialize_uservar(uint8_t *p, char *key, char *type, void *data,
+void bgenv_serialize_uservar(uint8_t *p, char *key, uint64_t type, void *data,
uint32_t record_size)
{
uint32_t payload_size, data_size;
@@ -78,20 +78,21 @@ void bgenv_serialize_uservar(uint8_t *p, char *key, char *type, void *data,
p += sizeof(uint32_t);

/* store datatype */
- memcpy(p, type, strlen(type) + 1);
- p += strlen(type) + 1;
+ *((uint64_t *)p) = type;
+ p += sizeof(uint64_t);

/* store data */
- data_size = payload_size - strlen(type) - 1 - sizeof(uint32_t);
+ data_size = payload_size - sizeof(uint32_t) - sizeof(uint64_t);
memcpy(p, data, data_size);
}

-int bgenv_get_uservar(uint8_t *udata, char *key, char *type, void *data,
+int bgenv_get_uservar(uint8_t *udata, char *key, uint64_t *type, void *data,
uint32_t maxlen)
{
uint8_t *uservar, *value;
- char *lkey, *ltype;
+ char *lkey;
uint32_t dsize;
+ uint64_t ltype;

uservar = bgenv_find_uservar(udata, key);

@@ -108,25 +109,24 @@ int bgenv_get_uservar(uint8_t *udata, char *key, char *type, void *data,
memcpy(data, value, dsize);

if (type) {
- memcpy(type, ltype, strlen(ltype) + 1);
+ *type = ltype;
}

return 0;
}

-int bgenv_set_uservar(uint8_t *udata, char *key, char *type, void *data,
+int bgenv_set_uservar(uint8_t *udata, char *key, uint64_t type, void *data,
uint32_t datalen)
{
uint32_t total_size;
uint8_t *p;

- total_size = datalen + strlen(type) + 1 + sizeof(uint32_t) +
+ total_size = datalen + sizeof(uint64_t) + sizeof(uint32_t) +
strlen(key) + 1;

p = bgenv_find_uservar(udata, key);
if (p) {
- if (strncmp(type, USERVAR_TYPE_DELETED,
- strlen(USERVAR_TYPE_DELETED) + 1) == 0) {
+ if (type & USERVAR_TYPE_DELETED) {
bgenv_del_uservar(udata, p);
return 0;
}
diff --git a/include/ebgenv.h b/include/ebgenv.h
index bb5b3c5..b569181 100644
--- a/include/ebgenv.h
+++ b/include/ebgenv.h
@@ -17,6 +17,21 @@

#include <errno.h>

+#define USERVAR_TYPE_CHAR 1
+#define USERVAR_TYPE_UINT8 2
+#define USERVAR_TYPE_UINT16 3
+#define USERVAR_TYPE_UINT32 4
+#define USERVAR_TYPE_UINT64 5
+#define USERVAR_TYPE_SINT8 6
+#define USERVAR_TYPE_SINT16 7
+#define USERVAR_TYPE_SINT32 8
+#define USERVAR_TYPE_SINT64 9
+#define USERVAR_TYPE_STRING_ASCII 32
+#define USERVAR_TYPE_BOOL 64
+#define USERVAR_TYPE_GLOBAL 1ULL << 62
+#define USERVAR_TYPE_DELETED 1ULL << 63
+#define USERVAR_TYPE_DEFAULT USERVAR_TYPE_GLOBAL
+
typedef struct {
void *bgenv;
bool ebg_new_env_created;
@@ -66,24 +81,23 @@ int ebg_env_set(ebgenv_t *e, char *key, char *value);
/** @brief Store new content into variable
* @param e A pointer to an ebgenv_t context.
* @param key name of the environment variable to set
- * @param datatype user specific string to identify the datatype of the value
+ * @param user specific or predefined datatype of the value
* @param value arbitrary data to be stored into the variable
* @param datalen length of the data to be stored into the variable
* @return 0 on success, -errno on failure
*/
-int ebg_env_set_ex(ebgenv_t *e, char *key, char *datatype, uint8_t *value,
+int ebg_env_set_ex(ebgenv_t *e, char *key, uint64_t datatype, uint8_t *value,
uint32_t datalen);

/** @brief Get content of user variable
* @param e A pointer to an ebgenv_t context.
* @param key name of the environment variable to retrieve
- * @param datatype buffer for user specific string to identify the
- * datatype of the value
+ * @param buffer to store the datatype of the value
* @param buffer destination for data to be stored into the variable
* @param maxlen size of provided buffer
* @return 0 on success, errno on failure
*/
-int ebg_env_get_ex(ebgenv_t *e, char *key, char *datatype, uint8_t *buffer,
+int ebg_env_get_ex(ebgenv_t *e, char *key, uint64_t *datatype, uint8_t *buffer,
uint32_t maxlen);

/** @brief Get available space for user variables
diff --git a/include/env_api.h b/include/env_api.h
index 5f535c5..8ea2c11 100644
--- a/include/env_api.h
+++ b/include/env_api.h
@@ -29,6 +29,7 @@
#include "config.h"
#include <zlib.h>
#include "envdata.h"
+#include "ebgenv.h"

#ifdef DEBUG
#define printf_debug(fmt, ...) printf(fmt, __VA_ARGS__)
@@ -44,9 +45,6 @@ extern bool bgenv_verbosity;
if (bgenv_verbosity) \
fprintf(o, __VA_ARGS__)

-#define USERVAR_TYPE_DEFAULT "String"
-#define USERVAR_TYPE_DELETED "\0xDE\0xAD"
-
typedef enum {
EBGENV_KERNELFILE,
EBGENV_KERNELPARAMS,
@@ -81,9 +79,9 @@ extern BG_ENVDATA *bgenv_read(BGENV *env);
extern bool bgenv_close(BGENV *env);

extern BGENV *bgenv_create_new(void);
-extern int bgenv_get(BGENV *env, char *key, char *type, void *data,
+extern int bgenv_get(BGENV *env, char *key, uint64_t *type, void *data,
uint32_t maxlen);
-extern int bgenv_set(BGENV *env, char *key, char *type, void *data,
+extern int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
uint32_t datalen);

#endif // __ENV_API_H__
diff --git a/include/uservars.h b/include/uservars.h
index d33bcce..b1d1d7c 100644
--- a/include/uservars.h
+++ b/include/uservars.h
@@ -15,14 +15,15 @@

#include <stdint.h>

-void bgenv_map_uservar(uint8_t *udata, char **key, char **type, uint8_t **val,
- uint32_t *record_size, uint32_t *data_size);
-void bgenv_serialize_uservar(uint8_t *p, char *key, char *type, void *data,
- uint32_t record_size);
+void bgenv_map_uservar(uint8_t *udata, char **key, uint64_t *type,
+ uint8_t **val, uint32_t *record_size,
+ uint32_t *data_size);
+void bgenv_serialize_uservar(uint8_t *p, char *key, uint64_t type, void *data,
+ uint32_t record_size);

-int bgenv_get_uservar(uint8_t *udata, char *key, char *type, void *data,
+int bgenv_get_uservar(uint8_t *udata, char *key, uint64_t *type, void *data,
uint32_t maxlen);
-int bgenv_set_uservar(uint8_t *udata, char *key, char *type, void *data,
+int bgenv_set_uservar(uint8_t *udata, char *key, uint64_t type, void *data,
uint32_t datalen);

uint8_t *bgenv_find_uservar(uint8_t *udata, char *key);
diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c
index 8aa6668..40a18c6 100644
--- a/tools/bg_setenv.c
+++ b/tools/bg_setenv.c
@@ -57,7 +57,7 @@ typedef enum { ENV_TASK_SET, ENV_TASK_DEL } BGENV_TASK;
struct stailhead *headp;
struct env_action {
char *key;
- char *type;
+ uint64_t type;
uint8_t *data;
BGENV_TASK task;
STAILQ_ENTRY(env_action) journal;
@@ -70,12 +70,11 @@ static void journal_free_action(struct env_action *action)
if (!action)
return;
free(action->data);
- free(action->type);
free(action->key);
free(action);
}

-static error_t journal_add_action(BGENV_TASK task, char *key, char *type,
+static error_t journal_add_action(BGENV_TASK task, char *key, uint64_t type,
uint8_t *data, size_t datalen)
{
struct env_action *new_action;
@@ -91,12 +90,7 @@ static error_t journal_add_action(BGENV_TASK task, char *key, char *type,
goto newaction_nomem;
}
}
- if (type) {
- if (asprintf(&(new_action->type), "%s", type) == -1) {
- new_action->type = NULL;
- goto newaction_nomem;
- }
- }
+ new_action->type = type;
if (data && datalen) {
new_action->data = (uint8_t *)malloc(datalen);
if (!new_action->data) {
@@ -121,7 +115,7 @@ static void journal_process_action(BGENV *env, struct env_action *action)

switch (action->task) {
case ENV_TASK_SET:
- VERBOSE(stdout, "Task = SET, key = %s, type = %s, val = %s\n",
+ VERBOSE(stdout, "Task = SET, key = %s, type = %llu, val = %s\n",
action->key, action->type, (char *)action->data);
if (strncmp(action->key, "ustate", strlen("ustate")+1) == 0) {
uint16_t ustate;
@@ -207,7 +201,7 @@ static error_t set_uservars(char *arg)

value = strtok(NULL, "=");
if (value == NULL) {
- return journal_add_action(ENV_TASK_DEL, key, NULL, NULL, 0);
+ return journal_add_action(ENV_TASK_DEL, key, 0, NULL, 0);
}
return journal_add_action(ENV_TASK_SET, key, USERVAR_TYPE_DEFAULT,
(uint8_t *)value, strlen(value) + 1);
@@ -229,7 +223,7 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
ENV_STRING_LENGTH);
return 1;
}
- e = journal_add_action(ENV_TASK_SET, "kernelfile", "String",
+ e = journal_add_action(ENV_TASK_SET, "kernelfile", 0,
(uint8_t *)arg, strlen(arg) + 1);
break;
case 'a':
@@ -240,7 +234,7 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
ENV_STRING_LENGTH);
return 1;
}
- e = journal_add_action(ENV_TASK_SET, "kernelparams", "String",
+ e = journal_add_action(ENV_TASK_SET, "kernelparams", 0,
(uint8_t *)arg, strlen(arg) + 1);
break;
case 'p':
@@ -287,7 +281,7 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
if (res == -1) {
return ENOMEM;
}
- e = journal_add_action(ENV_TASK_SET, "ustate", "String",
+ e = journal_add_action(ENV_TASK_SET, "ustate", 0,
(uint8_t *)tmp, strlen(tmp) + 1);
VERBOSE(stdout, "Ustate set to %d (%s).\n", i,
ustate2str(i));
@@ -296,7 +290,7 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
case 'r':
i = atoi(arg);
VERBOSE(stdout, "Revision is set to %d.\n", i);
- e = journal_add_action(ENV_TASK_SET, "revision", "String",
+ e = journal_add_action(ENV_TASK_SET, "revision", 0,
(uint8_t *)arg, strlen(arg) + 1);
break;
case 'w':
@@ -305,9 +299,8 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
VERBOSE(stdout,
"Setting watchdog timeout to %d seconds.\n", i);
e = journal_add_action(ENV_TASK_SET,
- "watchdog_timeout_sec",
- "String", (uint8_t *)arg,
- strlen(arg) + 1);
+ "watchdog_timeout_sec", 0,
+ (uint8_t *)arg, strlen(arg) + 1);
} else {
fprintf(stderr, "Watchdog timeout must be non-zero.\n");
return 1;
@@ -324,7 +317,7 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
VERBOSE(stdout,
"Confirming environment to work. Removing boot-once "
"and testing flag.\n");
- e = journal_add_action(ENV_TASK_SET, "ustate", "String",
+ e = journal_add_action(ENV_TASK_SET, "ustate", 0,
(uint8_t *)"0", 2);
break;
case 'u':
@@ -367,17 +360,18 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)

static void dump_uservars(uint8_t *udata)
{
- char *key, *value, *type;
+ char *key, *value;
+ uint64_t type;
uint32_t rsize, dsize;

while (*udata) {
bgenv_map_uservar(udata, &key, &type, (uint8_t **)&value,
&rsize, &dsize);
printf("%s ", key);
- if (strcmp(type, USERVAR_TYPE_DEFAULT) == 0) {
+ if ((type & 0x00000000FFFFFFFF) == USERVAR_TYPE_STRING_ASCII) {
printf("= %s\n", value);
} else {
- printf("( User defined type )\n");
+ printf("( Type is not ASCII )\n");
}
udata = bgenv_next_uservar(udata);
}
diff --git a/tools/tests/test_ebgenv_api.c b/tools/tests/test_ebgenv_api.c
index 3f44ea9..f0036a9 100644
--- a/tools/tests/test_ebgenv_api.c
+++ b/tools/tests/test_ebgenv_api.c
@@ -34,14 +34,15 @@ FAKE_VALUE_FUNC(bool, bgenv_init);
FAKE_VALUE_FUNC(bool, bgenv_write, BGENV *);
FAKE_VALUE_FUNC(bool, bgenv_close, BGENV *);

-int __real_bgenv_set(BGENV *, char *, char *, void *, uint32_t);
-int __wrap_bgenv_set(BGENV *, char *, char *, void *, uint32_t);
-int __real_bgenv_get(BGENV *, char *, char *, void *, uint32_t);
-int __wrap_bgenv_get(BGENV *, char *, char *, void *, uint32_t);
+int __real_bgenv_set(BGENV *, char *, uint64_t, void *, uint32_t);
+int __wrap_bgenv_set(BGENV *, char *, uint64_t, void *, uint32_t);
+int __real_bgenv_get(BGENV *, char *, uint64_t *, void *, uint32_t);
+int __wrap_bgenv_get(BGENV *, char *, uint64_t *, void *, uint32_t);

BGENV *bgenv_getset_arg0;
char *bgenv_getset_arg1;
-char *bgenv_getset_arg2;
+uint64_t *bgenv_get_arg2;
+uint64_t bgenv_set_arg2;
void *bgenv_getset_arg3;
uint32_t bgenv_getset_arg4;
int bgenv_get_call_count;
@@ -51,23 +52,25 @@ int bgenv_set_call_count;
* we need to use the linker wrapping method and reimplement some of FFFs
* functionality.
*/
-int __wrap_bgenv_get(BGENV *env, char *key, char *type, void *buffer, uint32_t len)
+int __wrap_bgenv_get(BGENV *env, char *key, uint64_t *type, void *buffer,
+ uint32_t len)
{
bgenv_get_call_count++;
bgenv_getset_arg0 = env;
bgenv_getset_arg1 = key;
- bgenv_getset_arg2 = type;
+ bgenv_get_arg2 = type;
bgenv_getset_arg3 = buffer;
bgenv_getset_arg4 = len;
return __real_bgenv_get(env, key, type, buffer, len);
}

-int __wrap_bgenv_set(BGENV *env, char *key, char *type, void *buffer, uint32_t len)
+int __wrap_bgenv_set(BGENV *env, char *key, uint64_t type, void *buffer,
+ uint32_t len)
{
bgenv_set_call_count++;
bgenv_getset_arg0 = env;
bgenv_getset_arg1 = key;
- bgenv_getset_arg2 = type;
+ bgenv_set_arg2 = type;
bgenv_getset_arg3 = buffer;
bgenv_getset_arg4 = len;
return __real_bgenv_set(env, key, type, buffer, len);
@@ -208,7 +211,7 @@ START_TEST(ebgenv_api_ebg_env_get)
ck_assert(bgenv_get_call_count == 1);
ck_assert(bgenv_getset_arg0 == e.bgenv);
ck_assert(bgenv_getset_arg1 == NULL);
- ck_assert(bgenv_getset_arg2 == NULL);
+ ck_assert(bgenv_get_arg2 == NULL);

/* Test if ebg_env_get retrieves correct data if given a valid
* environment handle.
@@ -270,7 +273,7 @@ START_TEST(ebgenv_api_ebg_env_set_ex)
memset(&e, 0, sizeof(e));
char *key = "mykey";
char *value = "dummy";
- char *usertype = "mytype";
+ uint64_t usertype = 1ULL << 36;
int32_t datalen = 5;

/* Check if ebg_env_set_ex correctly calls bgenv_set
@@ -290,7 +293,7 @@ START_TEST(ebgenv_api_ebg_env_set_ex)
ck_assert(bgenv_set_call_count == 1);
ck_assert(bgenv_getset_arg0 == e.bgenv);
ck_assert_int_eq(strcmp(bgenv_getset_arg1, key), 0);
- ck_assert_int_eq(strcmp(bgenv_getset_arg2, usertype), 0);
+ ck_assert_int_eq(bgenv_set_arg2, usertype);
ck_assert(bgenv_getset_arg3 == value);
ck_assert(bgenv_getset_arg4 == datalen);

@@ -305,7 +308,7 @@ START_TEST(ebgenv_api_ebg_env_get_ex)
memset(&e, 0, sizeof(e));
char *key = "mykey";
char buffer[5];
- char type[7];
+ uint64_t type;
int32_t datalen = 5;

/* Check if ebg_env_get_ex correctly calls bgenv_get
@@ -320,12 +323,12 @@ START_TEST(ebgenv_api_ebg_env_get_ex)

bgenv_get_call_count = 0;

- (void)ebg_env_get_ex(&e, key, type, (uint8_t *)buffer, datalen);
+ (void)ebg_env_get_ex(&e, key, &type, (uint8_t *)buffer, datalen);

ck_assert(bgenv_get_call_count == 1);
ck_assert(bgenv_getset_arg0 == e.bgenv);
ck_assert_int_eq(strcmp(bgenv_getset_arg1, key), 0);
- ck_assert(bgenv_getset_arg2 == type);
+ ck_assert(bgenv_get_arg2 == &type);
ck_assert(bgenv_getset_arg3 == buffer);
ck_assert(bgenv_getset_arg4 == datalen);

diff --git a/tools/tests/test_ebgenv_api_internal.c b/tools/tests/test_ebgenv_api_internal.c
index 1f2995b..25b9ff8 100644
--- a/tools/tests/test_ebgenv_api_internal.c
+++ b/tools/tests/test_ebgenv_api_internal.c
@@ -273,38 +273,38 @@ START_TEST(ebgenv_api_internal_bgenv_get)
handle->data->revision = 10000;
handle->data->ustate = USTATE_INSTALLED;

- char *type = NULL, *data = NULL;
+ char *data = NULL;
char buffera[22];
int res;

/* Test if bgenv_get fails if maxlen is set to 0
*/
- res = bgenv_get(handle, "kernelfile", type, data, 0);
+ res = bgenv_get(handle, "kernelfile", NULL, data, 0);
ck_assert_int_eq(res, -EINVAL);

/* Test if bgenv_get fails if key is NULL
*/
- res = bgenv_get(handle, NULL, type, data, 1000);
+ res = bgenv_get(handle, NULL, NULL, data, 1000);
ck_assert_int_eq(res, -EINVAL);

/* Test if bgenv_get fails if no environment is provided
*/
- res = bgenv_get(NULL, "kernelfile", type, NULL, 1000);
+ res = bgenv_get(NULL, "kernelfile", NULL, NULL, 1000);
ck_assert_int_eq(res, -EPERM);

/* Test if bgenv_get returns the correct size of the needed
* buffer if provided with a NULL buffer
*/
- res = bgenv_get(handle, "kernelfile", type, NULL, 1000);
+ res = bgenv_get(handle, "kernelfile", NULL, NULL, 1000);
ck_assert_int_eq(res, strlen(test_strings[0]) + 1);

/* Test if bgenv_get returns the correct value
*/
- res = bgenv_get(handle, "kernelfile", type, buffera, res);
+ res = bgenv_get(handle, "kernelfile", NULL, buffera, res);
ck_assert_int_eq(strcmp(buffera, test_strings[0]), 0);

- res = bgenv_get(handle, "kernelparams", type, NULL, 1000);
- res = bgenv_get(handle, "kernelparams", type, buffera, res);
+ res = bgenv_get(handle, "kernelparams", NULL, NULL, 1000);
+ res = bgenv_get(handle, "kernelparams", NULL, buffera, res);
ck_assert_int_eq(strcmp(buffera, test_strings[1]), 0);

free(handle);
@@ -321,17 +321,17 @@ START_TEST(ebgenv_api_internal_bgenv_set)

/* Test if bgenv_set returns -EINVAL if the handle is invalid
*/
- res = bgenv_set(NULL, "kernelfile", NULL, NULL, 0);
+ res = bgenv_set(NULL, "kernelfile", 0, NULL, 0);
ck_assert_int_eq(res, -EINVAL);

/* Test if bgenv_set returns -EINVAL if the key is invalid
*/
- res = bgenv_set(handle, "AOFIJAOEGIHA", NULL, NULL, 0);
+ res = bgenv_set(handle, "AOFIJAOEGIHA", 0, NULL, 0);
ck_assert_int_eq(res, -EINVAL);

/* Test if bgenv_set works correctly for valid parameters
*/
- res = bgenv_set(handle, "kernelfile", NULL, "vmlinuz", 8);
+ res = bgenv_set(handle, "kernelfile", 0, "vmlinuz", 8);
ck_assert_int_eq(res, 0);

char buffer[8];
@@ -339,31 +339,31 @@ START_TEST(ebgenv_api_internal_bgenv_set)

ck_assert(strcmp(kfile, "vmlinuz") == 0);

- res = bgenv_set(handle, "watchdog_timeout_sec", NULL, "-0", 2);
+ res = bgenv_set(handle, "watchdog_timeout_sec", 0, "-0", 2);
ck_assert_int_eq(res, 0);
ck_assert_int_eq(handle->data->watchdog_timeout_sec, 0);

- res = bgenv_set(handle, "watchdog_timeout_sec", NULL, "311", 4);
+ res = bgenv_set(handle, "watchdog_timeout_sec", 0, "311", 4);
ck_assert_int_eq(res, 0);
ck_assert_int_eq(handle->data->watchdog_timeout_sec, 311);

- res = bgenv_set(handle, "kernelparams", NULL, "root=", 6);
+ res = bgenv_set(handle, "kernelparams", 0, "root=", 6);
ck_assert_int_eq(res, 0);

char *kparm = str16to8(buffer, handle->data->kernelparams);

ck_assert(strcmp(kparm, "root=") == 0);

- res = bgenv_set(handle, "ustate", NULL, "2", 2);
+ res = bgenv_set(handle, "ustate", 0, "2", 2);
ck_assert_int_eq(res, 0);

ck_assert_int_eq(handle->data->ustate, 2);

- res = bgenv_set(handle, "revision", NULL, "0", 2);
+ res = bgenv_set(handle, "revision", 0, "0", 2);
ck_assert_int_eq(res, 0);
ck_assert_int_eq(handle->data->revision, 0);

- res = bgenv_set(handle, "revision", NULL, "10301", 6);
+ res = bgenv_set(handle, "revision", 0, "10301", 6);
ck_assert_int_eq(res, 0);
ck_assert_int_eq(handle->data->revision, 10301);

--
2.14.2

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

Include null terminating byte when calling bgenv_set.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

env/env_api.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/env/env_api.c b/env/env_api.c
index 8ee881a..76f7b93 100644
--- a/env/env_api.c
+++ b/env/env_api.c
@@ -176,7 +176,7 @@ int ebg_env_setglobalstate(ebgenv_t *e, uint16_t ustate)

(void)snprintf(buffer, sizeof(buffer), "%d", ustate);

res =
bgenv_set((BGENV *)e->bgenv, "ustate", "String", buffer,
- strlen(buffer));

+ strlen(buffer) + 1);

if (ustate != USTATE_OK) {
return res;

--
2.14.2

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

This patch set contains two fixes and a small API change to accomodate
swupdate's needs for global data storage.

It also adds tests for user variables.

Fixes:
* Deleting a user variable is done by setting its data type field to
USERVAR_TYPE_DELETED. The deletion function checks if the variable
exists and deletes it if this type is recognized. However, if the
variable was not found, the function created the variable instead with
this datatype stored into the environment. This is fixed so that
deleting non existing variables has no effect.
* ustate is set with the wrong data size, since the state is internally
handled as a null terminated string, the null terminating character
must also be paid attention at here.

Small API change:
* User types, which were not used yet except with deletion of variables,
were formerly introduced as a string. However, by defining it as a big
integer, flags as well as data type definitions can be handled much
better.
USERVAR_TYPE_DELETED is converted into bit 63 of the data type field,
which, if detected, causes deletion of the variable.
A new FLAG "USERVAR_TYPE_GLOBAL" is introduced, which gives a variable
the property "global" such that it is saved into all environment
copies. This implements a feature needed by swupdate, which wants to
store a temporary state which must not alter in failure cases or
rollback scenarios.
This flag is set per default so that all user variables are global
variables. If a user wants to create user variables that are managed
by the update mechanism, the functions ebg_env_set_ex / ebg_env_get_ex
can be used, to explicitely set the variable datatype and omit the
flag.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>

Andreas Reichel (5):
bugfix: ustate: include null termination calling bgenv_set
uservars: change type data field from char* to uint64_t
uservars: Implement USERVAR_TYPE_GLOBAL flag
Implement tests for user variables
bugfix: uservars: Fix deletion of deleted variables.

env/env_api.c | 9 ++-
env/env_api_fat.c | 43 ++++++++++---
env/uservars.c | 49 ++++++++-------
include/ebgenv.h | 24 ++++++--
include/env_api.h | 11 ++--
include/uservars.h | 13 ++--
tools/bg_setenv.c | 38 +++++-------
tools/tests/test_ebgenv_api.c | 33 +++++-----
tools/tests/test_ebgenv_api_internal.c | 109 +++++++++++++++++++++++++++------
9 files changed, 222 insertions(+), 107 deletions(-)

--
2.14.2

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

A user variable can be set to an empty string or it can be deleted. The
API distinguishes both cases using a USERVAR_TYPE_DELETED flag.

However, if a variable does not exist, and this flag is given, the
variable is wrongly created with the flag.

Fix the setter function so that if bgenv_find_uservar returns NULL, it
returns with no operation.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

env/uservars.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/env/uservars.c b/env/uservars.c
index f6013c1..791cce7 100644
--- a/env/uservars.c
+++ b/env/uservars.c
@@ -133,8 +133,11 @@ int bgenv_set_uservar(uint8_t *udata, char *key, uint64_t type, void *data,

p = bgenv_uservar_realloc(udata, total_size, p);
} else {
- p = bgenv_uservar_alloc(udata, total_size);
- }
+ if ((type & USERVAR_TYPE_DELETED) == 0) {
+ p = bgenv_uservar_alloc(udata, total_size);
+ } else {
+ return 0;
+ } }
if (!p) {
return -errno;
}
--
2.14.2

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

Include null terminating byte when calling bgenv_set.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

This patch set contains two fixes and a small API change to accomodate
swupdate's needs for global data storage.

It also adds tests for user variables.

Difference to v1: %llu expects long long unsigned int instead of
uint64_t, since long long unsigned int can be larger. Added a type cast
in bg_setenv.c.

include/ebgenv.h | 24 +++++--

include/env_api.h | 11 ++--
include/uservars.h | 13 ++--

tools/bg_setenv.c | 41 ++++++------
tools/tests/test_ebgenv_api.c | 33 +++++-----
tools/tests/test_ebgenv_api_internal.c | 112 +++++++++++++++++++++++++++------
9 files changed, 226 insertions(+), 109 deletions(-)

--
2.14.2

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

If the above flag is set in the type field of a user variable,
bgenv_set_uservar_global is called, which iterates all environments
and stores the given variable globally.

If the flag is absent, only the environment specified by the context
handle gets affected.

The flag is set per default if not otherwise specified.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

env/env_api_fat.c | 27 +++++++++++++++++++++++++++
include/env_api.h | 2 ++
2 files changed, 29 insertions(+)

diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 7d5e714..565ab44 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c
@@ -320,6 +320,29 @@ int bgenv_get(BGENV *env, char *key, uint64_t *type, void *data,
return 0;
}

+int bgenv_set_uservar_global(char *key, uint64_t type, void *data,
+ uint32_t datalen)
+{
+ int ret = 0;
+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
+ BGENV *env = bgenv_open_by_index(i);
+ if (!env) {
+ ret = -EIO;
+ continue;
+ }
+ int r = bgenv_set_uservar(env->data->userdata, key, type, data,
+ datalen);
+ if (r) {
+ ret = r;
+ }
+ if (!bgenv_write(env)) {
+ ret = -EIO;
+ }
+ (void)bgenv_close(env);
+ }
+ return ret;
+}
+
int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
uint32_t datalen)
{
@@ -337,6 +360,10 @@ int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
return -EPERM;
}
if (e == EBGENV_UNKNOWN) {
+ if (type & USERVAR_TYPE_GLOBAL) {
+ return bgenv_set_uservar_global(key, type, data,
+ datalen);
+ }
return bgenv_set_uservar(env->data->userdata, key, type, data,
datalen);
}
diff --git a/include/env_api.h b/include/env_api.h
index 8ea2c11..3b1ba1f 100644
--- a/include/env_api.h
+++ b/include/env_api.h
@@ -83,5 +83,7 @@ extern int bgenv_get(BGENV *env, char *key, uint64_t *type, void *data,
uint32_t maxlen);
extern int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
uint32_t datalen);
+extern int bgenv_set_uservar_global(char *key, uint64_t type,
+ void *data, uint32_t datalen);

#endif // __ENV_API_H__
--
2.14.2

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

Store user variable types as uint64_t instead of a string. This enables
the possibility of definint flags. Flags are used to delete a user
variable (to destinguish between setting it empty or removing it) and to
mark variables with special properties. This will be used in a following
commit to define global user variables.

Bits 0..31 are reserved for standard datatype definitions,
bits 32..48 are reserved for user defined datatypes or flags,
bits 49..63 are reserved for internal flags.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

env/env_api.c | 9 ++++----
env/env_api_fat.c | 16 +++++++------
env/uservars.c | 42 +++++++++++++++++-----------------
include/ebgenv.h | 24 +++++++++++++++----
include/env_api.h | 8 +++----
include/uservars.h | 13 ++++++-----

tools/bg_setenv.c | 41 +++++++++++++++------------------

tools/tests/test_ebgenv_api.c | 33 ++++++++++++++------------
tools/tests/test_ebgenv_api_internal.c | 34 +++++++++++++--------------

9 files changed, 116 insertions(+), 104 deletions(-)

diff --git a/env/env_api.c b/env/env_api.c
index 76f7b93..bd034af 100644
--- a/env/env_api.c
+++ b/env/env_api.c

@@ -99,7 +99,7 @@ int ebg_env_get(ebgenv_t *e, char *key, char *buffer)
ENV_STRING_LENGTH);
}

-int ebg_env_get_ex(ebgenv_t *e, char *key, char *usertype, uint8_t *buffer,
+int ebg_env_get_ex(ebgenv_t *e, char *key, uint64_t *usertype, uint8_t *buffer,
uint32_t maxlen)
{
return bgenv_get((BGENV *)e->bgenv, key, usertype, buffer, maxlen);
@@ -111,7 +111,7 @@ int ebg_env_set(ebgenv_t *e, char *key, char *value)
strlen(value) + 1);
}

-int ebg_env_set_ex(ebgenv_t *e, char *key, char *usertype, uint8_t *value,
+int ebg_env_set_ex(ebgenv_t *e, char *key, uint64_t usertype, uint8_t *value,
uint32_t datalen)
{
return bgenv_set((BGENV *)e->bgenv, key, usertype, value, datalen);

@@ -174,9 +174,8 @@ int ebg_env_setglobalstate(ebgenv_t *e, uint16_t ustate)
return EINVAL;

}
(void)snprintf(buffer, sizeof(buffer), "%d", ustate);

- res =
- bgenv_set((BGENV *)e->bgenv, "ustate", "String", buffer,
- strlen(buffer) + 1);
+ res = bgenv_set((BGENV *)e->bgenv, "ustate", 0, buffer,

+ strlen(buffer) + 1);

if (ustate != USTATE_OK) {
return res;

diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 8eb1454..7d5e714 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c

diff --git a/env/uservars.c b/env/uservars.c
index cd3f24b..f6013c1 100644
--- a/env/uservars.c
+++ b/env/uservars.c

+int bgenv_get_uservar(uint8_t *udata, char *key, uint64_t *type, void *data,
uint32_t maxlen)

{
uint8_t *uservar, *value;
- char *lkey, *ltype;
+ char *lkey;
uint32_t dsize;
+ uint64_t ltype;

uservar = bgenv_find_uservar(udata, key);

@@ -108,25 +109,24 @@ int bgenv_get_uservar(uint8_t *udata, char *key, char *type, void *data,
memcpy(data, value, dsize);

if (type) {
- memcpy(type, ltype, strlen(ltype) + 1);
+ *type = ltype;
}

return 0;
}

-int bgenv_set_uservar(uint8_t *udata, char *key, char *type, void *data,

+int bgenv_set_uservar(uint8_t *udata, char *key, uint64_t type, void *data,
uint32_t datalen)
{

diff --git a/include/env_api.h b/include/env_api.h
index 5f535c5..8ea2c11 100644
--- a/include/env_api.h
+++ b/include/env_api.h

@@ -29,6 +29,7 @@
#include "config.h"
#include <zlib.h>
#include "envdata.h"
+#include "ebgenv.h"

#ifdef DEBUG
#define printf_debug(fmt, ...) printf(fmt, __VA_ARGS__)
@@ -44,9 +45,6 @@ extern bool bgenv_verbosity;
if (bgenv_verbosity) \
fprintf(o, __VA_ARGS__)

-#define USERVAR_TYPE_DEFAULT "String"
-#define USERVAR_TYPE_DELETED "\0xDE\0xAD"
-
typedef enum {
EBGENV_KERNELFILE,
EBGENV_KERNELPARAMS,
@@ -81,9 +79,9 @@ extern BG_ENVDATA *bgenv_read(BGENV *env);
extern bool bgenv_close(BGENV *env);

extern BGENV *bgenv_create_new(void);
-extern int bgenv_get(BGENV *env, char *key, char *type, void *data,

+extern int bgenv_get(BGENV *env, char *key, uint64_t *type, void *data,
uint32_t maxlen);

-extern int bgenv_set(BGENV *env, char *key, char *type, void *data,
+extern int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
uint32_t datalen);

#endif // __ENV_API_H__
diff --git a/include/uservars.h b/include/uservars.h
index d33bcce..b1d1d7c 100644
--- a/include/uservars.h
+++ b/include/uservars.h
@@ -15,14 +15,15 @@

#include <stdint.h>

-void bgenv_map_uservar(uint8_t *udata, char **key, char **type, uint8_t **val,
- uint32_t *record_size, uint32_t *data_size);
-void bgenv_serialize_uservar(uint8_t *p, char *key, char *type, void *data,
- uint32_t record_size);
+void bgenv_map_uservar(uint8_t *udata, char **key, uint64_t *type,
+ uint8_t **val, uint32_t *record_size,
+ uint32_t *data_size);
+void bgenv_serialize_uservar(uint8_t *p, char *key, uint64_t type, void *data,
+ uint32_t record_size);

-int bgenv_get_uservar(uint8_t *udata, char *key, char *type, void *data,

+int bgenv_get_uservar(uint8_t *udata, char *key, uint64_t *type, void *data,
uint32_t maxlen);

-int bgenv_set_uservar(uint8_t *udata, char *key, char *type, void *data,

+int bgenv_set_uservar(uint8_t *udata, char *key, uint64_t type, void *data,
uint32_t datalen);

uint8_t *bgenv_find_uservar(uint8_t *udata, char *key);
diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c

index 8aa6668..a7b8a63 100644

@@ -121,8 +115,9 @@ static void journal_process_action(BGENV *env, struct env_action *action)

switch (action->task) {
case ENV_TASK_SET:
- VERBOSE(stdout, "Task = SET, key = %s, type = %s, val = %s\n",

- action->key, action->type, (char *)action->data);

+ VERBOSE(stdout, "Task = SET, key = %s, type = %llu, val = %s\n",

+ action->key, (long long unsigned int)action->type,
+ (char *)action->data);

if (strncmp(action->key, "ustate", strlen("ustate")+1) == 0) {
uint16_t ustate;

unsigned long t;
@@ -207,7 +202,7 @@ static error_t set_uservars(char *arg)

value = strtok(NULL, "=");
if (value == NULL) {
- return journal_add_action(ENV_TASK_DEL, key, NULL, NULL, 0);
+ return journal_add_action(ENV_TASK_DEL, key, 0, NULL, 0);
}
return journal_add_action(ENV_TASK_SET, key, USERVAR_TYPE_DEFAULT,
(uint8_t *)value, strlen(value) + 1);

@@ -229,7 +224,7 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)

ENV_STRING_LENGTH);
return 1;
}
- e = journal_add_action(ENV_TASK_SET, "kernelfile", "String",
+ e = journal_add_action(ENV_TASK_SET, "kernelfile", 0,
(uint8_t *)arg, strlen(arg) + 1);
break;
case 'a':

@@ -240,7 +235,7 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)

ENV_STRING_LENGTH);
return 1;
}
- e = journal_add_action(ENV_TASK_SET, "kernelparams", "String",
+ e = journal_add_action(ENV_TASK_SET, "kernelparams", 0,
(uint8_t *)arg, strlen(arg) + 1);
break;
case 'p':

@@ -287,7 +282,7 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)

if (res == -1) {
return ENOMEM;
}
- e = journal_add_action(ENV_TASK_SET, "ustate", "String",
+ e = journal_add_action(ENV_TASK_SET, "ustate", 0,
(uint8_t *)tmp, strlen(tmp) + 1);
VERBOSE(stdout, "Ustate set to %d (%s).\n", i,
ustate2str(i));

@@ -296,7 +291,7 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)

case 'r':
i = atoi(arg);
VERBOSE(stdout, "Revision is set to %d.\n", i);
- e = journal_add_action(ENV_TASK_SET, "revision", "String",
+ e = journal_add_action(ENV_TASK_SET, "revision", 0,
(uint8_t *)arg, strlen(arg) + 1);
break;
case 'w':

@@ -305,9 +300,8 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)

VERBOSE(stdout,
"Setting watchdog timeout to %d seconds.\n", i);
e = journal_add_action(ENV_TASK_SET,
- "watchdog_timeout_sec",
- "String", (uint8_t *)arg,
- strlen(arg) + 1);
+ "watchdog_timeout_sec", 0,
+ (uint8_t *)arg, strlen(arg) + 1);
} else {
fprintf(stderr, "Watchdog timeout must be non-zero.\n");
return 1;

@@ -324,7 +318,7 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)

VERBOSE(stdout,
"Confirming environment to work. Removing boot-once "
"and testing flag.\n");
- e = journal_add_action(ENV_TASK_SET, "ustate", "String",
+ e = journal_add_action(ENV_TASK_SET, "ustate", 0,
(uint8_t *)"0", 2);
break;
case 'u':

@@ -367,17 +361,18 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)

[Jan Kiszka]

Thanks, applied to next already.

Jan

--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

[Jan Kiszka]

On 2017-11-03 13:21, [ext] Andreas J. Reichel wrote:

I suppose the doc is now outdated. You specifically want to describe the
bit layout of type and predefined type codes.

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

Update for user variables and remove relict from swupdate-adapter.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

docs/API.md | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
docs/UPDATE.md | 16 +++++++++-------
2 files changed, 61 insertions(+), 10 deletions(-)

diff --git a/docs/API.md b/docs/API.md
index 1793581..e198b21 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -14,9 +14,7 @@ The interface provides functions to:
* edit the latest environment
* reset the error state
* check the last update state
-
-An example and detailed information on how to interpret the returned state values
-is given in the [swupdate-adapter documentation](../swupdate-adapter/swupdate.md).
+* manage user variables

To link a program to the library, you can install `efibootguard` with

@@ -31,6 +29,22 @@ If you want to cross-compile this library, you have to cross-compile the
`efibootguard tools` since both tools and this API library both depend on a
common static library. Refer to [compilation instructions](COMPILE.md).

+## User variables ##
+
+User variables are automatically set if the given variable key is not part of
+the internally pre-defined variable set. They reside in a special memory area
+of the environment managed by the API.
+
+The structure of an entry is explained in the [source code](../env/uservars.c).
+Also see the example program below.
+
+There are currently two types of user variables: Those with the flag
+USERVAR_TYPE_GLOBAL set in their data field and those without.
+
+Global variables are immune to the update process, i.e. they are stored into all
+environments. If not specifying flags manually with ebg_env_set_ex, the global
+flag is set per default.
+
## Example programs ##

The following example program creates a new environment with the latest revision
@@ -76,3 +90,38 @@ int main(void)
return 0;
}
```
+
+### Example on user variable usage ###
+
+```c
+#include <stdbool.h>
+#include "ebgenv.h"
+
+int main(void)
+{
+ ebgenv_t e;
+
+ ebg_env_open_current(&e);
+
+ /* This automatically creates a global user variable, stored in all
+ * environments */
+ ebg_env_set(&e, "myvar", "myvalue");
+
+ /* This sets the global variable to an empty string */
+ ebg_env_set(&e, "myvar", "");
+
+ /* This creates a user variable in the current environment only */
+ ebg_env_set_ex(&e, "currentvar", USERVAR_TYPE_STRING_ASCII, "abc",
+ strlen("abc") + 1);
+
+ /* This deletes the global myvar key from all environments.
+ * Note: value must not be NULL and datalen must be greater than 0 */
+ ebg_env_set_ex(&e, "myvar", USERVAR_TYPE_DELETED | USERVAR_TYPE_GLOBAL, "", 1);
+
+ /* This deletes the variable from the current environment */
+ ebg_env_set_ex(&e, "currentvar", USERVAR_TYPE_DELETED, "", 1);
+
+ ebg_env_close(&e);
+}
+
+```
diff --git a/docs/UPDATE.md b/docs/UPDATE.md
index bee5fd9..1cfeb71 100644
--- a/docs/UPDATE.md
+++ b/docs/UPDATE.md
@@ -9,13 +9,14 @@ The structure of the environment data is as follows:

```c
struct _BG_ENVDATA {
- uint16_t kernelfile[ENV_STRING_LENGTH];
- uint16_t kernelparams[ENV_STRING_LENGTH];
- uint8_t padding;
- uint8_t ustate;
- uint16_t watchdog_timeout_sec;
- uint32_t revision;
- uint32_t crc32;
+ uint16_t kernelfile[ENV_STRING_LENGTH];
+ uint16_t kernelparams[ENV_STRING_LENGTH];
+ uint8_t padding;
+ uint8_t ustate;
+ uint16_t watchdog_timeout_sec;
+ uint32_t revision;
+ uint8_t userdata[ENV_MEM_USERVARS];
+ uint32_t crc32;
};
```

@@ -27,6 +28,7 @@ The fields have the following meaning:
* `ustate`: Update status (`0` OK, `1` INSTALLED, `2` TESTING, `3`: FAILED)
* `watchdog_timeout_sec`: Number of seconds, the watchdog times out after
* `revision`: The revision number explained above
+* `userdata`: Stores user variables. See [API.md](API.md) for explanation.
* `crc32`: A crc32 checksum

The following example cases demonstrate the meaning of the update-specific
--
2.14.2

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

A user variable can be set to an empty string or it can be deleted. The
API distinguishes both cases using a USERVAR_TYPE_DELETED flag.

However, if a variable does not exist, and this flag is given, the
variable is wrongly created with the flag.

Fix the setter function so that if bgenv_find_uservar returns NULL, it
returns with no operation.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

env/uservars.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/env/uservars.c b/env/uservars.c
index 7e3dc26..c02bc14 100644
--- a/env/uservars.c
+++ b/env/uservars.c
@@ -140,8 +140,11 @@ int bgenv_set_uservar(uint8_t *udata, char *key, uint64_t type, void *data,

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

Store user variable types as uint64_t instead of a string. This enables
the possibility of definint flags. Flags are used to delete a user
variable (to destinguish between setting it empty or removing it) and to
mark variables with special properties. This will be used in a following
commit to define global user variables.

Bits 0..31 are reserved for standard datatype definitions,
bits 32..48 are reserved for user defined datatypes or flags,
bits 49..63 are reserved for internal flags.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

env/env_api.c | 9 +++----
env/env_api_fat.c | 16 ++++++-----
env/uservars.c | 49 +++++++++++++++++++---------------
include/ebgenv.h | 24 +++++++++++++----
include/env_api.h | 8 +++---
include/uservars.h | 13 ++++-----
tools/bg_setenv.c | 41 +++++++++++++---------------
tools/tests/test_ebgenv_api.c | 33 ++++++++++++-----------
tools/tests/test_ebgenv_api_internal.c | 34 +++++++++++------------
9 files changed, 123 insertions(+), 104 deletions(-)

diff --git a/env/uservars.c b/env/uservars.c
index cd3f24b..7e3dc26 100644
--- a/env/uservars.c
+++ b/env/uservars.c
@@ -14,21 +14,28 @@

#include "env_api.h"
#include "uservars.h"

-void bgenv_map_uservar(uint8_t *udata, char **key, char **type, uint8_t **val,
+void bgenv_map_uservar(uint8_t *udata, char **key, uint64_t *type, uint8_t **val,
uint32_t *record_size, uint32_t *data_size)
{
/* Each user variable is encoded as follows:
- * |------------|--------------|-------------|----------------|
- * | char KEY[] | uint32_t len | char type[] | uint8_t data[] |
- * |------------|--------------|-------------|----------------|
- * | KEY | < - - - - - - - - PAYLOAD - - - - - - - - > |
+ * |------------|--------------|---------------|----------------|
+ * | char KEY[] | uint32_t len | uint64_t type | uint8_t data[] |
+ * |------------|--------------|---------------|----------------|
+ * | KEY | < - - - - - - - - PAYLOAD - - - - - - - - - > |
*
* here char[] is a null-terminated string

* 'len' is the payload size (visualized by the horizontal dashes)

+ *
+ * type is partitioned into the following bit fields:
+ * | 63 ... 49 | 48 ... 32 | 31 ... 0 |
+ * | internal flags | user defined | standard types |
+ * | (reserved) | (free for user) | (reserved) |
+ *
+ * internal flags and standard types are declared in ebgenv.h

*/
char *var_key;
uint32_t *payload_size;
- char *var_type;
+ uint64_t *var_type;
uint8_t *data;

/* Get the key */

@@ -46,24 +53,24 @@ void bgenv_map_uservar(uint8_t *udata, char **key, char **type, uint8_t **val,

}

/* Get position of the type field */
- var_type = (char *)payload_size + sizeof(uint32_t);
+ var_type = (uint64_t *)((uint8_t *)payload_size + sizeof(uint32_t));
if (type) {
- *type = var_type;
+ *type = *var_type;
}

/* Calculate the data size */
if (data_size) {
*data_size = *payload_size - sizeof(uint32_t) -
- strlen(var_type) - 1;
+ sizeof(uint64_t);
}
/* Get the pointer to the data field */
- data = (uint8_t *)(var_type + strlen(var_type) + 1);
+ data = (uint8_t *)(var_type + sizeof(uint64_t));
if (val) {
*val = data;
}
}

-void bgenv_serialize_uservar(uint8_t *p, char *key, char *type, void *data,
+void bgenv_serialize_uservar(uint8_t *p, char *key, uint64_t type, void *data,
uint32_t record_size)
{
uint32_t payload_size, data_size;

@@ -78,20 +85,21 @@ void bgenv_serialize_uservar(uint8_t *p, char *key, char *type, void *data,

p += sizeof(uint32_t);

/* store datatype */
- memcpy(p, type, strlen(type) + 1);
- p += strlen(type) + 1;
+ *((uint64_t *)p) = type;
+ p += sizeof(uint64_t);

/* store data */
- data_size = payload_size - strlen(type) - 1 - sizeof(uint32_t);
+ data_size = payload_size - sizeof(uint32_t) - sizeof(uint64_t);
memcpy(p, data, data_size);
}

-int bgenv_get_uservar(uint8_t *udata, char *key, char *type, void *data,
+int bgenv_get_uservar(uint8_t *udata, char *key, uint64_t *type, void *data,
uint32_t maxlen)
{
uint8_t *uservar, *value;
- char *lkey, *ltype;
+ char *lkey;
uint32_t dsize;
+ uint64_t ltype;

uservar = bgenv_find_uservar(udata, key);

@@ -108,25 +116,24 @@ int bgenv_get_uservar(uint8_t *udata, char *key, char *type, void *data,

--
2.14.2

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

This patch set contains a fixes and a small API change to accomodate
swupdate's needs for global data storage and an update to the
documentation.

It also adds tests for user variables.

Fixes:
* Deleting a user variable is done by setting its data type field to
USERVAR_TYPE_DELETED. The deletion function checks if the variable
exists and deletes it if this type is recognized. However, if the
variable was not found, the function created the variable instead with
this datatype stored into the environment. This is fixed so that
deleting non existing variables has no effect.

Small API change:
* User types, which were not used yet except with deletion of variables,
were formerly introduced as a string. However, by defining it as a big
integer, flags as well as data type definitions can be handled much
better.
USERVAR_TYPE_DELETED is converted into bit 63 of the data type field,
which, if detected, causes deletion of the variable.
A new FLAG "USERVAR_TYPE_GLOBAL" is introduced, which gives a variable
the property "global" such that it is saved into all environment
copies. This implements a feature needed by swupdate, which wants to
store a temporary state which must not alter in failure cases or
rollback scenarios.
This flag is set per default so that all user variables are global
variables. If a user wants to create user variables that are managed
by the update mechanism, the functions ebg_env_set_ex / ebg_env_get_ex
can be used, to explicitely set the variable datatype and omit the
flag.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>

Andreas Reichel (5):

uservars: change type data field from char* to uint64_t
uservars: Implement USERVAR_TYPE_GLOBAL flag
Implement tests for user variables
bugfix: uservars: Fix deletion of deleted variables.

documentation: Update docs/API.md

docs/API.md | 55 +++++++++++++++-
docs/UPDATE.md | 16 ++---

env/env_api.c | 9 ++-
env/env_api_fat.c | 43 ++++++++++---

env/uservars.c | 56 ++++++++++-------

include/ebgenv.h | 24 +++++--
include/env_api.h | 11 ++--
include/uservars.h | 13 ++--
tools/bg_setenv.c | 41 ++++++------
tools/tests/test_ebgenv_api.c | 33 +++++-----
tools/tests/test_ebgenv_api_internal.c | 112 +++++++++++++++++++++++++++------

11 files changed, 294 insertions(+), 119 deletions(-)

--
2.14.2

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

If the above flag is set in the type field of a user variable,
bgenv_set_uservar_global is called, which iterates all environments
and stores the given variable globally.

If the flag is absent, only the environment specified by the context
handle gets affected.

The flag is set per default if not otherwise specified.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

env/env_api_fat.c | 27 +++++++++++++++++++++++++++
include/env_api.h | 2 ++
2 files changed, 29 insertions(+)

diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 7d5e714..565ab44 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c

@@ -320,6 +320,29 @@ int bgenv_get(BGENV *env, char *key, uint64_t *type, void *data,
return 0;
}

+int bgenv_set_uservar_global(char *key, uint64_t type, void *data,
+ uint32_t datalen)

+{
+ int ret = 0;
+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
+ BGENV *env = bgenv_open_by_index(i);
+ if (!env) {
+ ret = -EIO;
+ continue;
+ }
+ int r = bgenv_set_uservar(env->data->userdata, key, type, data,
+ datalen);
+ if (r) {
+ ret = r;
+ }
+ if (!bgenv_write(env)) {
+ ret = -EIO;
+ }
+ (void)bgenv_close(env);
+ }
+ return ret;
+}
+

int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
uint32_t datalen)

{
@@ -337,6 +360,10 @@ int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
return -EPERM;
}
if (e == EBGENV_UNKNOWN) {
+ if (type & USERVAR_TYPE_GLOBAL) {
+ return bgenv_set_uservar_global(key, type, data,
+ datalen);
+ }
return bgenv_set_uservar(env->data->userdata, key, type, data,
datalen);
}

diff --git a/include/env_api.h b/include/env_api.h
index 8ea2c11..3b1ba1f 100644
--- a/include/env_api.h
+++ b/include/env_api.h
@@ -83,5 +83,7 @@ extern int bgenv_get(BGENV *env, char *key, uint64_t *type, void *data,
uint32_t maxlen);

extern int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
uint32_t datalen);

+extern int bgenv_set_uservar_global(char *key, uint64_t type,

+ void *data, uint32_t datalen);

#endif // __ENV_API_H__
--
2.14.2

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

Implement test cases to check user variable storage with and without
USERVAR_TYPE_GLOBAL flag.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
include/env_api.h | 1 +
tools/tests/test_ebgenv_api_internal.c | 78 ++++++++++++++++++++++++++++++++--
2 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/include/env_api.h b/include/env_api.h
index 3b1ba1f..3433ac5 100644
--- a/include/env_api.h
+++ b/include/env_api.h
@@ -85,5 +85,6 @@ extern int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
uint32_t datalen);

extern int bgenv_set_uservar_global(char *key, uint64_t type,
void *data, uint32_t datalen);

+extern uint8_t *bgenv_find_uservar(uint8_t *userdata, char *key);

#endif // __ENV_API_H__
diff --git a/tools/tests/test_ebgenv_api_internal.c b/tools/tests/test_ebgenv_api_internal.c
index 25b9ff8..d826578 100644
--- a/tools/tests/test_ebgenv_api_internal.c
+++ b/tools/tests/test_ebgenv_api_internal.c
@@ -16,6 +16,7 @@
#include <env_api.h>
#include <env_config_file.h>
#include <env_config_partitions.h>
+#include <ebgenv.h>

DEFINE_FFF_GLOBALS;

@@ -23,9 +24,8 @@ static char *devpath = "/dev/nobrain";

Suite *ebg_test_suite(void);

-extern bool write_env(CONFIG_PART *part, BG_ENVDATA *env);
+extern bool write_env(CONFIG_PART *, BG_ENVDATA *);
extern EBGENVKEY bgenv_str2enum(char *);
-extern BGENV *bgenv_open_by_index(uint32_t index);

bool write_env_custom_fake(CONFIG_PART *part, BG_ENVDATA *env);

@@ -371,6 +371,77 @@ START_TEST(ebgenv_api_internal_bgenv_set)
}
END_TEST

+START_TEST(ebgenv_api_internal_uservars)
+{
+ RESET_FAKE(write_env);
+ write_env_fake.custom_fake = write_env_custom_fake;
+
+ int res;
+ BGENV *handle = bgenv_open_latest();
+
+ ck_assert(handle != NULL);
+ ck_assert(handle->data != NULL);
+
+ uint64_t type;
+ uint8_t *data;
+ /* Test a user variable using a user-defined data type
+ */
+ type = 1ULL << 36;
+ res = bgenv_set(handle, "myvar", type, "mydata", 5);
+ ck_assert_int_eq(res, 0);
+

+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {

+ data = bgenv_find_uservar((uint8_t *)&(envdata[i].userdata),
+ "myvar");
+ if (handle->data != &envdata[i]) {
+ ck_assert(data == NULL);
+ } else
+ {
+ ck_assert(data != NULL);
+ }
+ }
+
+ res = bgenv_set(handle, "myvar", USERVAR_TYPE_GLOBAL, "mydata", 5);
+ ck_assert_int_eq(write_env_fake.call_count, ENV_NUM_CONFIG_PARTS);
+ ck_assert_int_eq(res, 0);
+

+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {

+ data = bgenv_find_uservar((uint8_t *)&(envdata[i].userdata),
+ "myvar");
+ ck_assert(data != NULL);
+ }
+
+ res = bgenv_set(handle, "myvar", USERVAR_TYPE_DELETED, "mydata", 5);
+ ck_assert_int_eq(res, 0);
+

+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {

+ data = bgenv_find_uservar((uint8_t *)&(envdata[i].userdata),
+ "myvar");
+ if (handle->data != &envdata[i]) {
+ ck_assert(data != NULL);
+ } else
+ {
+ ck_assert(data == NULL);
+ }
+ }
+
+ write_env_fake.call_count = 0;
+
+ res = bgenv_set(handle, "myvar", USERVAR_TYPE_DELETED | USERVAR_TYPE_GLOBAL,
+ "mydata", 5);
+ ck_assert_int_eq(write_env_fake.call_count, ENV_NUM_CONFIG_PARTS);
+ ck_assert_int_eq(res, 0);
+

+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {

+ data = bgenv_find_uservar((uint8_t *)&(envdata[i].userdata),
+ "myvar");
+ ck_assert(data == NULL);
+ }
+
+ (void)bgenv_close(handle);
+}
+END_TEST
+
Suite *ebg_test_suite(void)
{
Suite *s;
@@ -388,7 +459,8 @@ Suite *ebg_test_suite(void)
ebgenv_api_internal_bgenv_read,
ebgenv_api_internal_bgenv_create_new,
ebgenv_api_internal_bgenv_get,
- ebgenv_api_internal_bgenv_set
+ ebgenv_api_internal_bgenv_set,
+ ebgenv_api_internal_uservars
};

tc_core = tcase_create("Core");
--
2.14.2

[Jan Kiszka]

On 2017-11-03 16:55, [ext] Andreas J. Reichel wrote:
> From: Andreas Reichel <andreas.r...@siemens.com>
>
> Store user variable types as uint64_t instead of a string. This enables
> the possibility of definint flags. Flags are used to delete a user
> variable (to destinguish between setting it empty or removing it) and to
> mark variables with special properties. This will be used in a following
> commit to define global user variables.
>
> Bits 0..31 are reserved for standard datatype definitions,
> bits 32..48 are reserved for user defined datatypes or flags,
> bits 49..63 are reserved for internal flags.
>
> Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>

...

> @@ -367,17 +361,18 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
>
> static void dump_uservars(uint8_t *udata)
> {
> - char *key, *value, *type;
> + char *key, *value;
> + uint64_t type;
> uint32_t rsize, dsize;
>
> while (*udata) {
> bgenv_map_uservar(udata, &key, &type, (uint8_t **)&value,
> &rsize, &dsize);
> printf("%s ", key);
> - if (strcmp(type, USERVAR_TYPE_DEFAULT) == 0) {
> + if ((type & 0x00000000FFFFFFFF) == USERVAR_TYPE_STRING_ASCII) {
> printf("= %s\n", value);
> } else {
> - printf("( User defined type )\n");
> + printf("( Type is not ASCII )\n");

Can't we also dump standard integer types? Can be done on top later on,
though.

[Jan Kiszka]

On 2017-11-03 16:55, [ext] Andreas J. Reichel wrote:

Style comments weren't addressed - will do on merge unless I find
something else.

[Jan Kiszka]

On 2017-11-03 16:55, [ext] Andreas J. Reichel wrote:

Hmm, why are you now using 3 spaces while it was 4 before and the
example code above uses 4?

Jan

> };
> ```
>
> @@ -27,6 +28,7 @@ The fields have the following meaning:
> * `ustate`: Update status (`0` OK, `1` INSTALLED, `2` TESTING, `3`: FAILED)
> * `watchdog_timeout_sec`: Number of seconds, the watchdog times out after
> * `revision`: The revision number explained above
> +* `userdata`: Stores user variables. See [API.md](API.md) for explanation.
> * `crc32`: A crc32 checksum
>
> The following example cases demonstrate the meaning of the update-specific
>

--

[Claudius Heine]

Hi,

Now when I see this I think that might be better and shorter written
like this:

1 << 32 - 1

Haven't done a review of the rest of the code, but that just poped up to
my eyes.

Cheers,
Claudius


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

[Jan Kiszka]

(1 << 32) - 1 then, to make it clearer.

>
> Haven't done a review of the rest of the code, but that just poped up to
> my eyes.
>

Waiting for your feedback as well then.

Thanks,

[Claudius Heine]

Hi,

Yes, you are right. Some type information/casts could be necessary as well.

Rest of this patch looks good to me.

[Jan Kiszka]

Maybe

#define USERVAR_STANDARD_TYPE_MASK ((1UL << 32) - 1)

?

Jan

>
> Rest of this patch looks good to me.
>
> Cheers,
> Claudius
>


--

[Claudius Heine]

Hi,

I don't like that return value is overwritten by succeeding error values
here. Maybe return on the first error would be better. Gracious
programming could lead to bad side effects.

If you have any reason for doing so:

ret = r ? r : ret;

looks better for me than:

if (r) {
ret = r;
}

But thats just subjective and very minor.

Cheers,
Claudius

> +
> int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
> uint32_t datalen)
> {
> @@ -337,6 +360,10 @@ int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
> return -EPERM;
> }
> if (e == EBGENV_UNKNOWN) {
> + if (type & USERVAR_TYPE_GLOBAL) {
> + return bgenv_set_uservar_global(key, type, data,
> + datalen);
> + }
> return bgenv_set_uservar(env->data->userdata, key, type, data,
> datalen);
> }
> diff --git a/include/env_api.h b/include/env_api.h
> index 8ea2c11..3b1ba1f 100644
> --- a/include/env_api.h
> +++ b/include/env_api.h
> @@ -83,5 +83,7 @@ extern int bgenv_get(BGENV *env, char *key, uint64_t *type, void *data,
> uint32_t maxlen);
> extern int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
> uint32_t datalen);
> +extern int bgenv_set_uservar_global(char *key, uint64_t type,
> + void *data, uint32_t datalen);
>
> #endif // __ENV_API_H__
>

--

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

If the above flag is set in the type field of a user variable,
bgenv_set_uservar_global is called, which iterates all environments
and stores the given variable globally.

If the flag is absent, only the environment specified by the context
handle gets affected.

The flag is set per default if not otherwise specified.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

env/env_api_fat.c | 36 ++++++++++++++++++++++++++++++++++++
include/env_api.h | 2 ++
2 files changed, 38 insertions(+)

diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 7d5e714..0aef867 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c
@@ -320,6 +320,38 @@ int bgenv_get(BGENV *env, char *key, uint64_t *type, void *data,

return 0;
}

+int bgenv_set_uservar_global(char *key, uint64_t type, void *data,
+ uint32_t datalen)
+{
+ int ret = 0;
+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {
+ BGENV *env = bgenv_open_by_index(i);
+ if (!env) {
+ ret = -EIO;

+ VERBOSE(stderr, "bgenv_set_uservar_global: Could not "
+ "open config partition #%d: (%s)\n",
+ i, strerror(-ret));

+ continue;
+ }
+ int r = bgenv_set_uservar(env->data->userdata, key, type, data,
+ datalen);
+ if (r) {

+ VERBOSE(stderr, "bgenv_set_uservar_global: Could not "
+ "set uservar on config partition "
+ " #%d: (%s)\n", i, strerror(-r));

+ ret = r;
+ }
+ if (!bgenv_write(env)) {
+ ret = -EIO;

+ VERBOSE(stderr, "bgenv_set_uservar_global: Could not "
+ " store environment to partition "
+ " #%d: (%s)\n", i, strerror(-ret));

+ }
+ (void)bgenv_close(env);
+ }
+ return ret;
+}

+
int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
uint32_t datalen)
{

@@ -337,6 +369,10 @@ int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,

return -EPERM;
}
if (e == EBGENV_UNKNOWN) {
+ if (type & USERVAR_TYPE_GLOBAL) {
+ return bgenv_set_uservar_global(key, type, data,
+ datalen);
+ }
return bgenv_set_uservar(env->data->userdata, key, type, data,
datalen);
}
diff --git a/include/env_api.h b/include/env_api.h
index 8ea2c11..3b1ba1f 100644
--- a/include/env_api.h
+++ b/include/env_api.h
@@ -83,5 +83,7 @@ extern int bgenv_get(BGENV *env, char *key, uint64_t *type, void *data,
uint32_t maxlen);
extern int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
uint32_t datalen);
+extern int bgenv_set_uservar_global(char *key, uint64_t type,
+ void *data, uint32_t datalen);

#endif // __ENV_API_H__
--

2.14.2

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

Update for user variables and remove relict from swupdate-adapter.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

docs/API.md | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
docs/UPDATE.md | 2 ++
2 files changed, 54 insertions(+), 3 deletions(-)

index bee5fd9..19c2f39 100644
--- a/docs/UPDATE.md
+++ b/docs/UPDATE.md
@@ -15,6 +15,7 @@ struct _BG_ENVDATA {
uint8_t ustate;
uint16_t watchdog_timeout_sec;

uint32_t revision;
+ uint8_t userdata[ENV_MEM_USERVARS];

uint32_t crc32;

};
```
@@ -27,6 +28,7 @@ The fields have the following meaning:
* `ustate`: Update status (`0` OK, `1` INSTALLED, `2` TESTING, `3`: FAILED)
* `watchdog_timeout_sec`: Number of seconds, the watchdog times out after
* `revision`: The revision number explained above
+* `userdata`: Stores user variables. See [API.md](API.md) for explanation.
* `crc32`: A crc32 checksum

The following example cases demonstrate the meaning of the update-specific
--

2.14.2

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

This patch set contains a fixes and a small API change to accomodate
swupdate's needs for global data storage and an update to the
documentation.

It also adds tests for user variables.

Fixes:
* Deleting a user variable is done by setting its data type field to
USERVAR_TYPE_DELETED. The deletion function checks if the variable
exists and deletes it if this type is recognized. However, if the
variable was not found, the function created the variable instead with
this datatype stored into the environment. This is fixed so that
deleting non existing variables has no effect.

* autotools: pkg-config was not checked correctly

Small API change:
* User types, which were not used yet except with deletion of variables,
were formerly introduced as a string. However, by defining it as a big
integer, flags as well as data type definitions can be handled much
better.
USERVAR_TYPE_DELETED is converted into bit 63 of the data type field,
which, if detected, causes deletion of the variable.
A new FLAG "USERVAR_TYPE_GLOBAL" is introduced, which gives a variable
the property "global" such that it is saved into all environment
copies. This implements a feature needed by swupdate, which wants to
store a temporary state which must not alter in failure cases or
rollback scenarios.
This flag is set per default so that all user variables are global
variables. If a user wants to create user variables that are managed
by the update mechanism, the functions ebg_env_set_ex / ebg_env_get_ex
can be used, to explicitely set the variable datatype and omit the

flag.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>

Andreas Reichel (6):

uservars: change type data field from char* to uint64_t
uservars: Implement USERVAR_TYPE_GLOBAL flag
Implement tests for user variables
bugfix: uservars: Fix deletion of deleted variables.
documentation: Update docs

configure.ac: Fix detection of pkg-config

configure.ac | 5 ++
docs/API.md | 55 +++++++++++++++-
docs/UPDATE.md | 2 +
env/env_api.c | 9 ++-
env/env_api_fat.c | 52 ++++++++++++---
env/uservars.c | 56 ++++++++++-------
include/ebgenv.h | 26 ++++++--

include/env_api.h | 11 ++--
include/uservars.h | 13 ++--

tools/bg_setenv.c | 89 +++++++++++++++++++-------

tools/tests/test_ebgenv_api.c | 33 +++++-----
tools/tests/test_ebgenv_api_internal.c | 112 +++++++++++++++++++++++++++------

12 files changed, 350 insertions(+), 113 deletions(-)

--
2.14.2

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

Store user variable types as uint64_t instead of a string. This enables
the possibility of definint flags. Flags are used to delete a user
variable (to destinguish between setting it empty or removing it) and to
mark variables with special properties. This will be used in a following
commit to define global user variables.

Bits 0..31 are reserved for standard datatype definitions,
bits 32..48 are reserved for user defined datatypes or flags,
bits 49..63 are reserved for internal flags.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

env/env_api.c | 9 ++--
env/env_api_fat.c | 16 +++---
env/uservars.c | 49 +++++++++++--------
include/ebgenv.h | 26 ++++++++--
include/env_api.h | 8 ++-
include/uservars.h | 13 ++---
tools/bg_setenv.c | 89 +++++++++++++++++++++++++---------
tools/tests/test_ebgenv_api.c | 33 +++++++------
tools/tests/test_ebgenv_api_internal.c | 34 ++++++-------
9 files changed, 172 insertions(+), 105 deletions(-)

diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 8eb1454..7d5e714 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c

+int bgenv_get_uservar(uint8_t *udata, char *key, uint64_t *type, void *data,
uint32_t maxlen)

{
uint8_t *uservar, *value;
- char *lkey, *ltype;
+ char *lkey;
uint32_t dsize;
+ uint64_t ltype;

uservar = bgenv_find_uservar(udata, key);

@@ -108,25 +116,24 @@ int bgenv_get_uservar(uint8_t *udata, char *key, char *type, void *data,
memcpy(data, value, dsize);

if (type) {
- memcpy(type, ltype, strlen(ltype) + 1);
+ *type = ltype;
}

return 0;
}

-int bgenv_set_uservar(uint8_t *udata, char *key, char *type, void *data,

+int bgenv_set_uservar(uint8_t *udata, char *key, uint64_t type, void *data,
uint32_t datalen)
{

uint32_t total_size;
uint8_t *p;

- total_size = datalen + strlen(type) + 1 + sizeof(uint32_t) +
+ total_size = datalen + sizeof(uint64_t) + sizeof(uint32_t) +
strlen(key) + 1;

p = bgenv_find_uservar(udata, key);
if (p) {
- if (strncmp(type, USERVAR_TYPE_DELETED,
- strlen(USERVAR_TYPE_DELETED) + 1) == 0) {
+ if (type & USERVAR_TYPE_DELETED) {
bgenv_del_uservar(udata, p);
return 0;
}
diff --git a/include/ebgenv.h b/include/ebgenv.h

index bb5b3c5..1257141 100644
--- a/include/ebgenv.h
+++ b/include/ebgenv.h
@@ -17,6 +17,23 @@

#include <errno.h>

+#define USERVAR_TYPE_CHAR 1
+#define USERVAR_TYPE_UINT8 2
+#define USERVAR_TYPE_UINT16 3
+#define USERVAR_TYPE_UINT32 4
+#define USERVAR_TYPE_UINT64 5
+#define USERVAR_TYPE_SINT8 6
+#define USERVAR_TYPE_SINT16 7
+#define USERVAR_TYPE_SINT32 8
+#define USERVAR_TYPE_SINT64 9
+#define USERVAR_TYPE_STRING_ASCII 32
+#define USERVAR_TYPE_BOOL 64
+#define USERVAR_TYPE_GLOBAL 1ULL << 62
+#define USERVAR_TYPE_DELETED 1ULL << 63
+#define USERVAR_TYPE_DEFAULT USERVAR_TYPE_GLOBAL
+

+#define USERVAR_STANDARD_TYPE_MASK ((1ULL << 32) - 1)

+
typedef struct {
void *bgenv;
bool ebg_new_env_created;

@@ -66,24 +83,23 @@ int ebg_env_set(ebgenv_t *e, char *key, char *value);

diff --git a/include/env_api.h b/include/env_api.h
index 5f535c5..8ea2c11 100644
--- a/include/env_api.h
+++ b/include/env_api.h

@@ -29,6 +29,7 @@
#include "config.h"
#include <zlib.h>
#include "envdata.h"
+#include "ebgenv.h"

#ifdef DEBUG
#define printf_debug(fmt, ...) printf(fmt, __VA_ARGS__)
@@ -44,9 +45,6 @@ extern bool bgenv_verbosity;
if (bgenv_verbosity) \
fprintf(o, __VA_ARGS__)

-#define USERVAR_TYPE_DEFAULT "String"
-#define USERVAR_TYPE_DELETED "\0xDE\0xAD"
-
typedef enum {
EBGENV_KERNELFILE,
EBGENV_KERNELPARAMS,
@@ -81,9 +79,9 @@ extern BG_ENVDATA *bgenv_read(BGENV *env);
extern bool bgenv_close(BGENV *env);

extern BGENV *bgenv_create_new(void);
-extern int bgenv_get(BGENV *env, char *key, char *type, void *data,

+extern int bgenv_get(BGENV *env, char *key, uint64_t *type, void *data,
uint32_t maxlen);
-extern int bgenv_set(BGENV *env, char *key, char *type, void *data,
+extern int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
uint32_t datalen);

#endif // __ENV_API_H__

diff --git a/include/uservars.h b/include/uservars.h
index d33bcce..b1d1d7c 100644
--- a/include/uservars.h
+++ b/include/uservars.h
@@ -15,14 +15,15 @@

#include <stdint.h>

-void bgenv_map_uservar(uint8_t *udata, char **key, char **type, uint8_t **val,
- uint32_t *record_size, uint32_t *data_size);
-void bgenv_serialize_uservar(uint8_t *p, char *key, char *type, void *data,
- uint32_t record_size);
+void bgenv_map_uservar(uint8_t *udata, char **key, uint64_t *type,
+ uint8_t **val, uint32_t *record_size,
+ uint32_t *data_size);
+void bgenv_serialize_uservar(uint8_t *p, char *key, uint64_t type, void *data,
+ uint32_t record_size);

-int bgenv_get_uservar(uint8_t *udata, char *key, char *type, void *data,

+int bgenv_get_uservar(uint8_t *udata, char *key, uint64_t *type, void *data,
uint32_t maxlen);

-int bgenv_set_uservar(uint8_t *udata, char *key, char *type, void *data,

+int bgenv_set_uservar(uint8_t *udata, char *key, uint64_t type, void *data,
uint32_t datalen);

uint8_t *bgenv_find_uservar(uint8_t *udata, char *key);
diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c

index 8aa6668..b438841 100644

@@ -367,18 +361,65 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)

static void dump_uservars(uint8_t *udata)
{
- char *key, *value, *type;
+ char *key, *value;
+ uint64_t type;
uint32_t rsize, dsize;

+ uint64_t val_unum;
+ int64_t val_snum;

while (*udata) {
bgenv_map_uservar(udata, &key, &type, (uint8_t **)&value,
&rsize, &dsize);
printf("%s ", key);
- if (strcmp(type, USERVAR_TYPE_DEFAULT) == 0) {

+ type &= USERVAR_STANDARD_TYPE_MASK;
+ if (type == USERVAR_TYPE_STRING_ASCII) {

printf("= %s\n", value);

- } else {

- printf("( User defined type )\n");

+ } else
+ if (type >= USERVAR_TYPE_UINT8 && type <= USERVAR_TYPE_UINT64) {
+ switch(type) {
+ case USERVAR_TYPE_UINT8:
+ val_unum = *((uint8_t *) value);
+ break;
+ case USERVAR_TYPE_UINT16:
+ val_unum = *((uint16_t *) value);
+ break;
+ case USERVAR_TYPE_UINT32:
+ val_unum = *((uint32_t *) value);
+ break;
+ case USERVAR_TYPE_UINT64:
+ val_unum = *((uint64_t *) value);
+ break;
+ }
+ printf("= %llu\n", (long long unsigned int) val_unum);
+ } else
+ if (type >= USERVAR_TYPE_SINT8 && type <= USERVAR_TYPE_SINT64) {
+ switch(type) {
+ case USERVAR_TYPE_SINT8:
+ val_snum = *((int8_t *) value);
+ break;
+ case USERVAR_TYPE_SINT16:
+ val_snum = *((int16_t *) value);
+ break;
+ case USERVAR_TYPE_SINT32:
+ val_snum = *((int32_t *) value);
+ break;
+ case USERVAR_TYPE_SINT64:
+ val_snum = *((int64_t *) value);
+ break;
+ }
+ printf("= %lld\n", (long long signed int) val_snum);
+ } else
+ switch(type) {
+ case USERVAR_TYPE_CHAR:
+ printf("= %c\n", (char) *value);
+ break;
+ case USERVAR_TYPE_BOOL:
+ printf("= %s\n", (bool) *value ? "true" : "false");
+ break;
+ default:
+ printf("( Type is not printable )\n");
}
+

diff --git a/tools/tests/test_ebgenv_api_internal.c b/tools/tests/test_ebgenv_api_internal.c
index 1f2995b..25b9ff8 100644
--- a/tools/tests/test_ebgenv_api_internal.c
+++ b/tools/tests/test_ebgenv_api_internal.c

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

A user variable can be set to an empty string or it can be deleted. The
API distinguishes both cases using a USERVAR_TYPE_DELETED flag.

However, if a variable does not exist, and this flag is given, the
variable is wrongly created with the flag.

Fix the setter function so that if bgenv_find_uservar returns NULL, it
returns with no operation.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

env/uservars.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/env/uservars.c b/env/uservars.c
index 7e3dc26..c02bc14 100644
--- a/env/uservars.c
+++ b/env/uservars.c

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

Implement test cases to check user variable storage with and without

USERVAR_TYPE_GLOBAL flag.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>

---
include/env_api.h | 1 +
tools/tests/test_ebgenv_api_internal.c | 78 ++++++++++++++++++++++++++++++++--
2 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/include/env_api.h b/include/env_api.h
index 3b1ba1f..3433ac5 100644
--- a/include/env_api.h
+++ b/include/env_api.h

@@ -85,5 +85,6 @@ extern int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
uint32_t datalen);
extern int bgenv_set_uservar_global(char *key, uint64_t type,
void *data, uint32_t datalen);
+extern uint8_t *bgenv_find_uservar(uint8_t *userdata, char *key);

#endif // __ENV_API_H__
diff --git a/tools/tests/test_ebgenv_api_internal.c b/tools/tests/test_ebgenv_api_internal.c
index 25b9ff8..d826578 100644
--- a/tools/tests/test_ebgenv_api_internal.c
+++ b/tools/tests/test_ebgenv_api_internal.c

+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {

+ data = bgenv_find_uservar((uint8_t *)&(envdata[i].userdata),
+ "myvar");
+ if (handle->data != &envdata[i]) {
+ ck_assert(data == NULL);
+ } else
+ {
+ ck_assert(data != NULL);
+ }
+ }
+
+ res = bgenv_set(handle, "myvar", USERVAR_TYPE_GLOBAL, "mydata", 5);
+ ck_assert_int_eq(write_env_fake.call_count, ENV_NUM_CONFIG_PARTS);
+ ck_assert_int_eq(res, 0);
+

+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {

+ data = bgenv_find_uservar((uint8_t *)&(envdata[i].userdata),
+ "myvar");
+ ck_assert(data != NULL);
+ }
+
+ res = bgenv_set(handle, "myvar", USERVAR_TYPE_DELETED, "mydata", 5);
+ ck_assert_int_eq(res, 0);
+

+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {

+ data = bgenv_find_uservar((uint8_t *)&(envdata[i].userdata),
+ "myvar");
+ if (handle->data != &envdata[i]) {
+ ck_assert(data != NULL);
+ } else
+ {
+ ck_assert(data == NULL);
+ }
+ }
+
+ write_env_fake.call_count = 0;
+
+ res = bgenv_set(handle, "myvar", USERVAR_TYPE_DELETED | USERVAR_TYPE_GLOBAL,
+ "mydata", 5);
+ ck_assert_int_eq(write_env_fake.call_count, ENV_NUM_CONFIG_PARTS);
+ ck_assert_int_eq(res, 0);
+

+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {

+ data = bgenv_find_uservar((uint8_t *)&(envdata[i].userdata),
+ "myvar");
+ ck_assert(data == NULL);
+ }
+
+ (void)bgenv_close(handle);
+}
+END_TEST
+
Suite *ebg_test_suite(void)
{
Suite *s;
@@ -388,7 +459,8 @@ Suite *ebg_test_suite(void)
ebgenv_api_internal_bgenv_read,
ebgenv_api_internal_bgenv_create_new,
ebgenv_api_internal_bgenv_get,
- ebgenv_api_internal_bgenv_set
+ ebgenv_api_internal_bgenv_set,
+ ebgenv_api_internal_uservars
};

tc_core = tcase_create("Core");

--
2.14.2

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

If pkg-config is not present, the configure script produces
a misleading syntax error, because the PKG_CHECK_MODULES function
is not defined. The already integrated check seems to fail on some
platforms, thus add another check if the executable is there.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

configure.ac | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/configure.ac b/configure.ac
index 7e8e7e9..d9a719c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -164,6 +164,11 @@ AC_ARG_WITH([mem-uservars],

AC_DEFINE_UNQUOTED([ENV_MEM_USERVARS], [${ENV_MEM_USERVARS}], [Reserved memory for user variables])

+dnl pkg-config
+AC_PATH_PROG(PKG_CONFIG, pkg-config, no)
+if test "x$PKG_CONFIG" = "xno"; then
+ AC_MSG_ERROR([You need to install pkg-config])
+fi
PKG_PROG_PKG_CONFIG()
PKG_CHECK_MODULES(LIBCHECK, check)
# ------------------------------------------------------------------------------
--
2.14.2

[Claudius Heine]

Hi,

Not sure what 'VERBOSE' should stand for. Is is more for debugging
messages or for any message?

I would expect that errors that occurred should be printed out
regardless of the logging level.

Claudius

[Andreas Reichel]

So far in the API we only printed something if the user activated
logging. If we want to change this, we should do that in an extra patch
for consistency reasons.

Andreas

--
Andreas Reichel
Dipl.-Phys. (Univ.)
Software Consultant

Andreas...@tngtech.com, +49-174-3180074
TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterfoehring
Geschaeftsfuehrer: Henrik Klagges, Dr. Robert Dahlke, Gerhard Mueller
Sitz: Unterfoehring * Amtsgericht Muenchen * HRB 135082

[Andreas Reichel]

On Tue, Nov 07, 2017 at 03:12:24PM +0100, Andreas J. Reichel wrote:
> From: Andreas Reichel <andreas.r...@siemens.com>
>

Hi Jan, can we merge this?
Claudius and I agreed.

Kind regards

[Jan Kiszka]

Fixing up the coding style here while merging to keep things readable.
Please use the right indention level and also avoid misleading

} else
if (...

in the future.

Thanks
Jan

[Jan Kiszka]

On 2017-11-07 15:12, [ext] Andreas J. Reichel wrote:

This test fails when applying only up to this patch. Can I fold patch 4
into this one? It fixes the bug.

Jan

[Jan Kiszka]

Never mind, will keep the split.

All patches merged to next, thanks.

Jan

[Claudius Heine]

Hi,

Should we invest some work to integrate a style checker into travis?

-- Claudius

[Jan Kiszka]

As there are always reasonable exceptions from strict styles, I'm
reluctant to have a binary check here. Maybe there are rules that can be
always enforced, but identifying them takes some effort.

Jan

[Claudius Heine]

Hi,

My argument there is that review should not be about checking if the
style fits the personal taste at that moment, because reviewing if the
right about of whitespaces were placed at the correct position would
distract me from reviewing the implementation of the code.

I am for a strict style checker, just because it avoids discussion about
it. But I am against correcting styles automatically, because the
developer should think themselves about how to structure the code for
the best reading experience but within the rules of the style guide.

clang-format is a bit of a strange tool for that, because checking
styles with it is not straight forward. Maybe there are other tools
around that provide a similar experience to what we have with
pep8/pycodestyle or pylint in python.

Cheers,

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

Update for user variables and remove relict from swupdate-adapter.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

2.15.0

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

If the above flag is set in the type field of a user variable,
bgenv_set_uservar_global is called, which iterates all environments
and stores the given variable globally.

If the flag is absent, only the environment specified by the context
handle gets affected.

The flag is set per default if not otherwise specified.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

env/env_api_fat.c | 36 ++++++++++++++++++++++++++++++++++++
include/env_api.h | 2 ++

tools/bg_setenv.c | 9 +++++++++
3 files changed, 47 insertions(+)

diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 7d5e714..0aef867 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c

@@ -320,6 +320,38 @@ int bgenv_get(BGENV *env, char *key, uint64_t *type, void *data,
return 0;
}

+int bgenv_set_uservar_global(char *key, uint64_t type, void *data,
+ uint32_t datalen)

+{
+ int ret = 0;

+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {

+ BGENV *env = bgenv_open_by_index(i);
+ if (!env) {
+ ret = -EIO;
+ VERBOSE(stderr, "bgenv_set_uservar_global: Could not "
+ "open config partition #%d: (%s)\n",
+ i, strerror(-ret));

+ continue;
+ }
+ int r = bgenv_set_uservar(env->data->userdata, key, type, data,
+ datalen);
+ if (r) {
+ VERBOSE(stderr, "bgenv_set_uservar_global: Could not "
+ "set uservar on config partition "
+ " #%d: (%s)\n", i, strerror(-r));
+ ret = r;
+ }
+ if (!bgenv_write(env)) {
+ ret = -EIO;
+ VERBOSE(stderr, "bgenv_set_uservar_global: Could not "
+ " store environment to partition "
+ " #%d: (%s)\n", i, strerror(-ret));
+ }
+ (void)bgenv_close(env);
+ }
+ return ret;
+}
+

int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
uint32_t datalen)

{
@@ -337,6 +369,10 @@ int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
return -EPERM;
}
if (e == EBGENV_UNKNOWN) {
+ if (type & USERVAR_TYPE_GLOBAL) {
+ return bgenv_set_uservar_global(key, type, data,
+ datalen);
+ }
return bgenv_set_uservar(env->data->userdata, key, type, data,
datalen);
}

diff --git a/include/env_api.h b/include/env_api.h
index 8ea2c11..3b1ba1f 100644
--- a/include/env_api.h
+++ b/include/env_api.h
@@ -83,5 +83,7 @@ extern int bgenv_get(BGENV *env, char *key, uint64_t *type, void *data,
uint32_t maxlen);

extern int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
uint32_t datalen);

+extern int bgenv_set_uservar_global(char *key, uint64_t type,
+ void *data, uint32_t datalen);

#endif // __ENV_API_H__
diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c
index 819c762..bc65c51 100644
--- a/tools/bg_setenv.c
+++ b/tools/bg_setenv.c
@@ -65,6 +65,8 @@ struct env_action {

STAILQ_HEAD(stailhead, env_action) head = STAILQ_HEAD_INITIALIZER(head);

+static bool inhibit_global_store = false;
+

static void journal_free_action(struct env_action *action)
{
if (!action)

@@ -113,6 +115,12 @@ static void journal_process_action(BGENV *env, struct env_action *action)
ebgenv_t e;
char *tmp;

+ if (inhibit_global_store) {
+ /* If the environment is written to a file, we don't want a
+ * global variable storage, but only into the output file */
+ action->type &= ~USERVAR_TYPE_GLOBAL;
+ }
+

switch (action->task) {
case ENV_TASK_SET:

VERBOSE(stdout, "Task = SET, key = %s, type = %llu, val = %s\n",

@@ -309,6 +317,7 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
break;
case 'f':
arguments->output_to_file = true;
+ inhibit_global_store = true;
res = asprintf(&envfilepath, "%s/%s", arg, FAT_ENV_FILENAME);

if (res == -1) {
return ENOMEM;

--
2.15.0

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

Store user variable types as uint64_t instead of a string. This enables
the possibility of definint flags. Flags are used to delete a user
variable (to destinguish between setting it empty or removing it) and to
mark variables with special properties. This will be used in a following
commit to define global user variables.

Bits 0..31 are reserved for standard datatype definitions,
bits 32..48 are reserved for user defined datatypes or flags,
bits 49..63 are reserved for internal flags.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

env/env_api.c | 9 ++--
env/env_api_fat.c | 16 +++---
env/uservars.c | 49 +++++++++++--------
include/ebgenv.h | 26 ++++++++--
include/env_api.h | 8 ++-
include/uservars.h | 13 ++---
tools/bg_setenv.c | 89 +++++++++++++++++++++++++---------
tools/tests/test_ebgenv_api.c | 33 +++++++------
tools/tests/test_ebgenv_api_internal.c | 34 ++++++-------

9 files changed, 173 insertions(+), 104 deletions(-)

diff --git a/env/env_api_fat.c b/env/env_api_fat.c
index 8eb1454..7d5e714 100644
--- a/env/env_api_fat.c
+++ b/env/env_api_fat.c

index cd3f24b..02ed1d2 100644

+ data = (uint8_t *)var_type + sizeof(uint64_t);

if (val) {
*val = data;
}
}

-void bgenv_serialize_uservar(uint8_t *p, char *key, char *type, void *data,
+void bgenv_serialize_uservar(uint8_t *p, char *key, uint64_t type, void *data,
uint32_t record_size)
{
uint32_t payload_size, data_size;
@@ -78,20 +85,21 @@ void bgenv_serialize_uservar(uint8_t *p, char *key, char *type, void *data,
p += sizeof(uint32_t);

/* store datatype */
- memcpy(p, type, strlen(type) + 1);
- p += strlen(type) + 1;
+ *((uint64_t *)p) = type;
+ p += sizeof(uint64_t);

/* store data */
- data_size = payload_size - strlen(type) - 1 - sizeof(uint32_t);
+ data_size = payload_size - sizeof(uint32_t) - sizeof(uint64_t);
memcpy(p, data, data_size);
}

-int bgenv_get_uservar(uint8_t *udata, char *key, char *type, void *data,

+int bgenv_get_uservar(uint8_t *udata, char *key, uint64_t *type, void *data,
uint32_t maxlen)

{
uint8_t *uservar, *value;
- char *lkey, *ltype;
+ char *lkey;
uint32_t dsize;
+ uint64_t ltype;

uservar = bgenv_find_uservar(udata, key);

@@ -108,25 +116,24 @@ int bgenv_get_uservar(uint8_t *udata, char *key, char *type, void *data,
memcpy(data, value, dsize);

if (type) {
- memcpy(type, ltype, strlen(ltype) + 1);
+ *type = ltype;
}

return 0;
}

-int bgenv_set_uservar(uint8_t *udata, char *key, char *type, void *data,

+int bgenv_set_uservar(uint8_t *udata, char *key, uint64_t type, void *data,
uint32_t datalen)
{

diff --git a/include/env_api.h b/include/env_api.h
index 5f535c5..8ea2c11 100644
--- a/include/env_api.h
+++ b/include/env_api.h

@@ -29,6 +29,7 @@
#include "config.h"
#include <zlib.h>
#include "envdata.h"
+#include "ebgenv.h"

#ifdef DEBUG
#define printf_debug(fmt, ...) printf(fmt, __VA_ARGS__)
@@ -44,9 +45,6 @@ extern bool bgenv_verbosity;
if (bgenv_verbosity) \
fprintf(o, __VA_ARGS__)

-#define USERVAR_TYPE_DEFAULT "String"
-#define USERVAR_TYPE_DELETED "\0xDE\0xAD"
-
typedef enum {
EBGENV_KERNELFILE,
EBGENV_KERNELPARAMS,
@@ -81,9 +79,9 @@ extern BG_ENVDATA *bgenv_read(BGENV *env);
extern bool bgenv_close(BGENV *env);

extern BGENV *bgenv_create_new(void);
-extern int bgenv_get(BGENV *env, char *key, char *type, void *data,

+extern int bgenv_get(BGENV *env, char *key, uint64_t *type, void *data,
uint32_t maxlen);
-extern int bgenv_set(BGENV *env, char *key, char *type, void *data,
+extern int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,

uint32_t datalen);

#endif // __ENV_API_H__

diff --git a/include/uservars.h b/include/uservars.h
index d33bcce..b1d1d7c 100644
--- a/include/uservars.h
+++ b/include/uservars.h
@@ -15,14 +15,15 @@

#include <stdint.h>

-void bgenv_map_uservar(uint8_t *udata, char **key, char **type, uint8_t **val,
- uint32_t *record_size, uint32_t *data_size);
-void bgenv_serialize_uservar(uint8_t *p, char *key, char *type, void *data,
- uint32_t record_size);
+void bgenv_map_uservar(uint8_t *udata, char **key, uint64_t *type,
+ uint8_t **val, uint32_t *record_size,
+ uint32_t *data_size);
+void bgenv_serialize_uservar(uint8_t *p, char *key, uint64_t type, void *data,
+ uint32_t record_size);

-int bgenv_get_uservar(uint8_t *udata, char *key, char *type, void *data,

+int bgenv_get_uservar(uint8_t *udata, char *key, uint64_t *type, void *data,
uint32_t maxlen);

-int bgenv_set_uservar(uint8_t *udata, char *key, char *type, void *data,

+int bgenv_set_uservar(uint8_t *udata, char *key, uint64_t type, void *data,
uint32_t datalen);

uint8_t *bgenv_find_uservar(uint8_t *udata, char *key);

diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c
index 8aa6668..819c762 100644
--- a/tools/bg_setenv.c
+++ b/tools/bg_setenv.c

@@ -57,7 +57,7 @@ typedef enum { ENV_TASK_SET, ENV_TASK_DEL } BGENV_TASK;
struct stailhead *headp;
struct env_action {
char *key;
- char *type;
+ uint64_t type;
uint8_t *data;
BGENV_TASK task;
STAILQ_ENTRY(env_action) journal;

@@ -70,12 +70,11 @@ static void journal_free_action(struct env_action *action)
if (!action)

return;
free(action->data);
- free(action->type);
free(action->key);
free(action);
}

-static error_t journal_add_action(BGENV_TASK task, char *key, char *type,
+static error_t journal_add_action(BGENV_TASK task, char *key, uint64_t type,
uint8_t *data, size_t datalen)
{
struct env_action *new_action;
@@ -91,12 +90,7 @@ static error_t journal_add_action(BGENV_TASK task, char *key, char *type,
goto newaction_nomem;
}
}
- if (type) {
- if (asprintf(&(new_action->type), "%s", type) == -1) {
- new_action->type = NULL;
- goto newaction_nomem;
- }
- }
+ new_action->type = type;
if (data && datalen) {
new_action->data = (uint8_t *)malloc(datalen);
if (!new_action->data) {

@@ -121,8 +115,9 @@ static void journal_process_action(BGENV *env, struct env_action *action)

switch (action->task) {
case ENV_TASK_SET:

- VERBOSE(stdout, "Task = SET, key = %s, type = %s, val = %s\n",

- action->key, action->type, (char *)action->data);

+ VERBOSE(stdout, "Task = SET, key = %s, type = %llu, val = %s\n",

@@ -287,7 +282,7 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)

if (res == -1) {
return ENOMEM;
}

@@ -367,18 +361,67 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)

static void dump_uservars(uint8_t *udata)
{
- char *key, *value, *type;
+ char *key, *value;
+ uint64_t type;
uint32_t rsize, dsize;
+ uint64_t val_unum;
+ int64_t val_snum;

while (*udata) {
bgenv_map_uservar(udata, &key, &type, (uint8_t **)&value,
&rsize, &dsize);
printf("%s ", key);
- if (strcmp(type, USERVAR_TYPE_DEFAULT) == 0) {
+ type &= USERVAR_STANDARD_TYPE_MASK;
+ if (type == USERVAR_TYPE_STRING_ASCII) {
printf("= %s\n", value);

+ } else if (type >= USERVAR_TYPE_UINT8 &&
+ type <= USERVAR_TYPE_UINT64) {

+ switch(type) {
+ case USERVAR_TYPE_UINT8:
+ val_unum = *((uint8_t *) value);
+ break;
+ case USERVAR_TYPE_UINT16:
+ val_unum = *((uint16_t *) value);
+ break;
+ case USERVAR_TYPE_UINT32:
+ val_unum = *((uint32_t *) value);
+ break;
+ case USERVAR_TYPE_UINT64:
+ val_unum = *((uint64_t *) value);
+ break;
+ }
+ printf("= %llu\n", (long long unsigned int) val_unum);

+ } else if (type >= USERVAR_TYPE_SINT8 &&
+ type <= USERVAR_TYPE_SINT64) {

+ switch(type) {
+ case USERVAR_TYPE_SINT8:
+ val_snum = *((int8_t *) value);
+ break;
+ case USERVAR_TYPE_SINT16:
+ val_snum = *((int16_t *) value);
+ break;
+ case USERVAR_TYPE_SINT32:
+ val_snum = *((int32_t *) value);
+ break;
+ case USERVAR_TYPE_SINT64:
+ val_snum = *((int64_t *) value);
+ break;
+ }
+ printf("= %lld\n", (long long signed int) val_snum);

} else {
- printf("( User defined type )\n");

+ switch(type) {

+ case USERVAR_TYPE_CHAR:
+ printf("= %c\n", (char) *value);
+ break;
+ case USERVAR_TYPE_BOOL:
+ printf("= %s\n",

+ (bool) *value ? "true" : "false");

+ break;
+ default:
+ printf("( Type is not printable )\n");

+ }

diff --git a/tools/tests/test_ebgenv_api_internal.c b/tools/tests/test_ebgenv_api_internal.c
index 1f2995b..25b9ff8 100644
--- a/tools/tests/test_ebgenv_api_internal.c
+++ b/tools/tests/test_ebgenv_api_internal.c

2.15.0

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

Implement test cases to check user variable storage with and without
USERVAR_TYPE_GLOBAL flag.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

include/env_api.h | 1 +
tools/tests/test_ebgenv_api_internal.c | 78 ++++++++++++++++++++++++++++++++--
2 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/include/env_api.h b/include/env_api.h
index 3b1ba1f..3433ac5 100644
--- a/include/env_api.h
+++ b/include/env_api.h
@@ -85,5 +85,6 @@ extern int bgenv_set(BGENV *env, char *key, uint64_t type, void *data,
uint32_t datalen);

extern int bgenv_set_uservar_global(char *key, uint64_t type,
void *data, uint32_t datalen);
+extern uint8_t *bgenv_find_uservar(uint8_t *userdata, char *key);

#endif // __ENV_API_H__
diff --git a/tools/tests/test_ebgenv_api_internal.c b/tools/tests/test_ebgenv_api_internal.c
index 25b9ff8..d826578 100644
--- a/tools/tests/test_ebgenv_api_internal.c
+++ b/tools/tests/test_ebgenv_api_internal.c

+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {

+ data = bgenv_find_uservar((uint8_t *)&(envdata[i].userdata),
+ "myvar");
+ if (handle->data != &envdata[i]) {
+ ck_assert(data == NULL);
+ } else
+ {
+ ck_assert(data != NULL);
+ }
+ }
+
+ res = bgenv_set(handle, "myvar", USERVAR_TYPE_GLOBAL, "mydata", 5);
+ ck_assert_int_eq(write_env_fake.call_count, ENV_NUM_CONFIG_PARTS);
+ ck_assert_int_eq(res, 0);
+

+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {

+ data = bgenv_find_uservar((uint8_t *)&(envdata[i].userdata),
+ "myvar");
+ ck_assert(data != NULL);
+ }
+
+ res = bgenv_set(handle, "myvar", USERVAR_TYPE_DELETED, "mydata", 5);
+ ck_assert_int_eq(res, 0);
+

+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {

+ data = bgenv_find_uservar((uint8_t *)&(envdata[i].userdata),
+ "myvar");
+ if (handle->data != &envdata[i]) {
+ ck_assert(data != NULL);
+ } else
+ {
+ ck_assert(data == NULL);
+ }
+ }
+
+ write_env_fake.call_count = 0;
+
+ res = bgenv_set(handle, "myvar", USERVAR_TYPE_DELETED | USERVAR_TYPE_GLOBAL,
+ "mydata", 5);
+ ck_assert_int_eq(write_env_fake.call_count, ENV_NUM_CONFIG_PARTS);
+ ck_assert_int_eq(res, 0);
+

+ for (int i = 0; i < ENV_NUM_CONFIG_PARTS; i++) {

+ data = bgenv_find_uservar((uint8_t *)&(envdata[i].userdata),
+ "myvar");
+ ck_assert(data == NULL);
+ }
+
+ (void)bgenv_close(handle);
+}
+END_TEST
+
Suite *ebg_test_suite(void)
{
Suite *s;
@@ -388,7 +459,8 @@ Suite *ebg_test_suite(void)
ebgenv_api_internal_bgenv_read,
ebgenv_api_internal_bgenv_create_new,
ebgenv_api_internal_bgenv_get,
- ebgenv_api_internal_bgenv_set
+ ebgenv_api_internal_bgenv_set,
+ ebgenv_api_internal_uservars
};

tc_core = tcase_create("Core");

--
2.15.0

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

This patch set contains a fixes and a small API change to accomodate
swupdate's needs for global data storage and an update to the
documentation.

It also adds tests for user variables.

Fixes:
* Deleting a user variable is done by setting its data type field to
USERVAR_TYPE_DELETED. The deletion function checks if the variable
exists and deletes it if this type is recognized. However, if the
variable was not found, the function created the variable instead with
this datatype stored into the environment. This is fixed so that
deleting non existing variables has no effect.

Small API change:
* User types, which were not used yet except with deletion of variables,
were formerly introduced as a string. However, by defining it as a big
integer, flags as well as data type definitions can be handled much
better.
USERVAR_TYPE_DELETED is converted into bit 63 of the data type field,
which, if detected, causes deletion of the variable.
A new FLAG "USERVAR_TYPE_GLOBAL" is introduced, which gives a variable
the property "global" such that it is saved into all environment
copies. This implements a feature needed by swupdate, which wants to
store a temporary state which must not alter in failure cases or
rollback scenarios.
This flag is set per default so that all user variables are global
variables. If a user wants to create user variables that are managed
by the update mechanism, the functions ebg_env_set_ex / ebg_env_get_ex
can be used, to explicitely set the variable datatype and omit the
flag.

Tools:
* When using the -f option with bg_setenv, the default type for user
variables is masked to never be global. Otherwise, other environments
might be changed and no user variables get stored into the file.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>

Andreas Reichel (6):
uservars: change type data field from char* to uint64_t
uservars: Implement USERVAR_TYPE_GLOBAL flag

bugfix: uservars: Fix deletion of deleted variables.
Implement tests for user variables
documentation: Update docs
configure.ac: Fix detection of pkg-config

configure.ac | 5 ++
docs/API.md | 55 +++++++++++++++-
docs/UPDATE.md | 2 +
env/env_api.c | 9 ++-
env/env_api_fat.c | 52 ++++++++++++---

env/uservars.c | 55 +++++++++-------

include/ebgenv.h | 26 ++++++--
include/env_api.h | 11 ++--
include/uservars.h | 13 ++--

tools/bg_setenv.c | 98 ++++++++++++++++++++++-------

tools/tests/test_ebgenv_api.c | 33 +++++-----
tools/tests/test_ebgenv_api_internal.c | 112 +++++++++++++++++++++++++++------

12 files changed, 360 insertions(+), 111 deletions(-)

--
2.15.0

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

A user variable can be set to an empty string or it can be deleted. The
API distinguishes both cases using a USERVAR_TYPE_DELETED flag.

However, if a variable does not exist, and this flag is given, the
variable is wrongly created with the flag.

Fix the setter function so that if bgenv_find_uservar returns NULL, it
returns with no operation.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

env/uservars.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/env/uservars.c b/env/uservars.c
index 02ed1d2..4952624 100644
--- a/env/uservars.c
+++ b/env/uservars.c
@@ -140,7 +140,11 @@ int bgenv_set_uservar(uint8_t *udata, char *key, uint64_t type, void *data,

p = bgenv_uservar_realloc(udata, total_size, p);
} else {
- p = bgenv_uservar_alloc(udata, total_size);

+ if ((type & USERVAR_TYPE_DELETED) == 0) {
+ p = bgenv_uservar_alloc(udata, total_size);
+ } else {
+ return 0;
+ }
}
if (!p) {
return -errno;
--

2.15.0

[Andreas J. Reichel]

From: Andreas Reichel <andreas.r...@siemens.com>

If pkg-config is not present, the configure script produces
a misleading syntax error, because the PKG_CHECK_MODULES function
is not defined. The already integrated check seems to fail on some
platforms, thus add another check if the executable is there.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

configure.ac | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/configure.ac b/configure.ac
index 7e8e7e9..d9a719c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -164,6 +164,11 @@ AC_ARG_WITH([mem-uservars],

AC_DEFINE_UNQUOTED([ENV_MEM_USERVARS], [${ENV_MEM_USERVARS}], [Reserved memory for user variables])

+dnl pkg-config
+AC_PATH_PROG(PKG_CONFIG, pkg-config, no)
+if test "x$PKG_CONFIG" = "xno"; then
+ AC_MSG_ERROR([You need to install pkg-config])
+fi
PKG_PROG_PKG_CONFIG()
PKG_CHECK_MODULES(LIBCHECK, check)
# ------------------------------------------------------------------------------
--

2.15.0

[Andreas Reichel]

On Tue, Nov 14, 2017 at 02:20:38PM +0100, Andreas J. Reichel wrote:
> From: Andreas Reichel <andreas.r...@siemens.com>

This patch series is thought as replacement for the one already applied
to next. It fixes a bug with a wrong pointer arithmetic noticed by
coverity scan.
Also, patches 3 and 4 are swapped so that checks are possible in
between.

Sorry for the mistake.

[Jan Kiszka]

Thanks, applied to next as replacement of v4.

[Jan Kiszka]

On 2017-11-14 14:20, [ext] Andreas J. Reichel wrote:

> From: Andreas Reichel <andreas.r...@siemens.com>
>
> If the above flag is set in the type field of a user variable,
> bgenv_set_uservar_global is called, which iterates all environments
> and stores the given variable globally.
>
> If the flag is absent, only the environment specified by the context
> handle gets affected.
>
> The flag is set per default if not otherwise specified.

Could you follow-up with a patch that documents the flag semantics in
include/ebgenv.h?

Thanks,

[Jan Kiszka]

On 2017-11-14 14:20, [ext] Andreas J. Reichel wrote:

> A new FLAG "USERVAR_TYPE_GLOBAL" is introduced, which gives a variable
> the property "global" such that it is saved into all environment
> copies. This implements a feature needed by swupdate, which wants to
> store a temporary state which must not alter in failure cases or
> rollback scenarios.

Hopefully, you can resolve this concern quickly:

Setting a global variable during each update process (this is my
understanding of this swupdate desire) now means not only changing the
config partition of the upcoming version but also modifying the
existing, working one. Which measures ensure that we never corrupt the
current config, e.g. due to a power loss in the middle of a write?

So far, we only modified the upcoming version (config and data), which
even put us ahead of U-boot-based switch-over implementations. It now
appears to me that we are regressing here, no? If so, could we address
this be protecting user variables separately from the critical EFI boot
guard state?

[Andreas Reichel]

I see your point. There is always a non-vanishing chance to corrupt data
if writing to it. By implementing the global recovery state for SWUpdate
the idea was to edit all user variable spaces of all environments.
However, as you state, there is a chance that there is a power loss
between the sync system call and the controller writing the actual data.
Although this is very small, as the environment is written within micro
seconds, it should not be disregarded.

Also, when chosing a separate region within the same environment file,
with a separate CRC, the complete file may be corrupted. The same is
true if writing to an adjacent block by not using the file system, since
the controller could write the whole track internally instead of just
the block. The best way i.m.o. to increase security on this is to create a
second area for user variables, for the global ones, which is far enough
away from the sensitive evironment, i.e. a reserved block of the FAT
file system. Then local user variables are stored into the same
environment as before and global user variables are stored directly
to disk away from the BGENV.DAT file.

The only remaining chance of corruption is due to the write-access to
the disk itself.

One might also consider using UEFI variables for storing the state.
However, due to the fragile implementation on some systems and even
accessing them from kernel user space requiring specific kernel
configuration settings, I would skip UEFI variables as a solution.

Andreas

> Jan
>
> --
> Siemens AG, Corporate Technology, CT RDA ITP SES-DE
> Corporate Competence Center Embedded Linux