[PATCH] env_api: Delete recovery_status user variable for SWUpdate

[Andreas J. Reichel]

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

If 'recovery_status' is set to 'in_progress' by the bootloader interface
of SWUpdate, then the api sets the internal environment struct member
called 'in_progress' to 1 by creating a new environment. However, if
SWUpdate sets it to 'failed', then SWUpdate's bootloader interface calls
'ebg_env_set' with a variable name of 'recovery_status', which creates a
user variable with that name.
After an update process, the SWUpdate bootloader interface's unset
function is called which must unset the 'recovery_status'. However, now
depending on wether it was changed to 'in_progress' or 'failed' before,
the user variable might exist or not. This logic is complex and much
easier to implement into env_api by deleting such a user variable if it
exists inside the finalize function.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
env/env_api.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/env/env_api.c b/env/env_api.c
index 75695d1..bf5c7c9 100644
--- a/env/env_api.c
+++ b/env/env_api.c
@@ -240,11 +240,23 @@ int ebg_env_close(ebgenv_t *e)
return 0;
}

-int ebg_env_finalize_update(ebgenv_t *e) {
+int ebg_env_finalize_update(ebgenv_t *e)
+{
if (!e->bgenv || !((BGENV *)e->bgenv)->data) {
return EIO;
}
- ((BGENV *)e->bgenv)->data->in_progress = 0;
- ((BGENV *)e->bgenv)->data->ustate = USTATE_INSTALLED;
+
+ BG_ENVDATA *data;
+ data = ((BGENV *)e->bgenv)->data;
+ data->in_progress = 0;
+ data->ustate = USTATE_INSTALLED;
+
+ /* Also delete any ramaining user variable called 'recovery_status' */
+ uint8_t *recv_status;
+ recv_status = bgenv_find_uservar(data->userdata, "recovery_status");
+ if (recv_status) {
+ bgenv_del_uservar(data->userdata, recv_status);
+ }
+
return 0;
}
--
2.16.2

[Jan Kiszka]

remaining

Giving a brief reason here again might help event more than describing
what is done below...

> + uint8_t *recv_status;
> + recv_status = bgenv_find_uservar(data->userdata, "recovery_status");
> + if (recv_status) {
> + bgenv_del_uservar(data->userdata, recv_status);
> + }
> +
> return 0;
> }
>

But doesn't that encode the behavior of one user of our API into our
API? Sounds not like I should like this. Or even merge it.

Jan

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

[Andreas Reichel]

It's true that this is rather SWUpdate specific. I could say that we
have developed the API mainly for SWUpdate but of course that cannot
deny that a generic API should be free of user-specific code that is
not generally applicable.

But what do you think of the following idea:

treat the finalize API call as kind of a destructor for a pre-registered
set of user variables, then the API could provide such a register
function and SWUpdate can - as any software using the API - specify
a list of user variables that must be garbage collected (deleted) upon
call of ebg_env_finalize_update. This way we

* don't have SWUpdate specific code in here
* specify a method that SWUpdate can use to solve its problem
* provide a user-friendly method to clean up user variable space
with temporal variables needed only by the update process

Andreas

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

--
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

[Jan Kiszka]

Sounds better to me, indeed.