| Message ID | 20190717082525.891-12-takahiro.akashi@linaro.org |
|---|---|
| State | RFC |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | efi_loader: non-volatile variables support | expand |
Dear Takahiro, In message <20190717082525.891-12-takahiro.akashi@linaro.org> you wrote: > By definition, when any variable with VARSTORAGE_NON_VOLATILE_AUTOSAVE > is modified with env_save_ext(), the associated context should be written > back to storage immediately. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > cmd/nvedit.c | 34 ++++++++++++++++++++++++++++++++-- > 1 file changed, 32 insertions(+), 2 deletions(-) > > diff --git a/cmd/nvedit.c b/cmd/nvedit.c > index cc80ba712767..9896a4141a2a 100644 > --- a/cmd/nvedit.c > +++ b/cmd/nvedit.c > @@ -271,8 +271,27 @@ static int _do_env_set(int flag, int argc, char * const argv[], int env_flag, > > /* Delete only ? */ > if (argc < 3 || argv[2] == NULL) { > - int rc = hdelete_r(name, &env_htab, env_flag); > - return !rc; > + uint32_t flags; > + > + int rc = hdelete_ext(name, &env_htab, ctx, &flags, env_flag); > + if (!rc) { > + debug("Failed to delete from hash table\n"); > + return 1; > + } > + > + if (env_flags_parse_varstorage_from_binflags(flags) > + == env_flags_varstorage_non_volatile_autosave) { > + int ret; > + > + ret = env_save_ext(ctx); > + if (ret) { > + printf("## Error saving variables, ret = %d\n", > + ret); > + return 1; > + } > + } > + > + return 0; I think your logic of using hdelete() here to provide flag information is not correct. The existing code stores flag information independent of the actual existence of the variable. Your code breaks this. Best regards, Wolfgang Denk
diff --git a/cmd/nvedit.c b/cmd/nvedit.c index cc80ba712767..9896a4141a2a 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -271,8 +271,27 @@ static int _do_env_set(int flag, int argc, char * const argv[], int env_flag, /* Delete only ? */ if (argc < 3 || argv[2] == NULL) { - int rc = hdelete_r(name, &env_htab, env_flag); - return !rc; + uint32_t flags; + + int rc = hdelete_ext(name, &env_htab, ctx, &flags, env_flag); + if (!rc) { + debug("Failed to delete from hash table\n"); + return 1; + } + + if (env_flags_parse_varstorage_from_binflags(flags) + == env_flags_varstorage_non_volatile_autosave) { + int ret; + + ret = env_save_ext(ctx); + if (ret) { + printf("## Error saving variables, ret = %d\n", + ret); + return 1; + } + } + + return 0; } /* @@ -307,6 +326,17 @@ static int _do_env_set(int flag, int argc, char * const argv[], int env_flag, return 1; } + if (env_flags_parse_varstorage_from_binflags(ep->flags) + == env_flags_varstorage_non_volatile_autosave) { + int ret; + + ret = env_save_ext(ep->context); + if (ret) { + printf("## Error saving variables, ret = %d\n", ret); + return 1; + } + } + return 0; }
By definition, when any variable with VARSTORAGE_NON_VOLATILE_AUTOSAVE is modified with env_save_ext(), the associated context should be written back to storage immediately. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- cmd/nvedit.c | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-)