| Message ID | 3a0d972a-f06b-40ca-8cae-fa52af360afb@googlegroups.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [libubootenv] uboot_env: Fix handling of .flags variable when it contains variables which are not created yet | expand |
Hi Stefano, can this patch be applied ? We are using it in our products since a while, but would like to have it part of the community version. Or is there something else you need to be prepared? Uwe Am Freitag, 2. August 2019 13:49:44 UTC+2 schrieb Uwe Schuster: > > This tries to fix the problem with variable access or type flags which are > part of the .flags variable but do not exist as a env variable as reported > by me in https://groups.google.com/d/msg/swupdate/ShdD10dl7Pc/ULUYSpFDCAAJ > : > > 1. The write once (o) access atrribute flag is not properly handled if the >> variable referenced by the .flags value string does not exist yet (as it >> would be the case with write once variables before they are first written >> or set). Instead this variables flags gets removed from .flags value and >> attributes are lost. >> This behavior also applies to all other variables which do not exist yet, >> but in case of write once variables I'd consider it a normal use case. >> > > I had to choose an implementation and I chose to put such uncreated > variables into the metadata database as normal variables but with NULL > value assigned. > > The patch also fixes a problem with the existing parsing function. It > failed to handle .flags value strings with more then one variable (comma > separated). > > The only known problem what remains is with fw_printenv function. It does > currently not print out the .flags variable (as normal u-boot would). To > fix that more changes to the printenv implementation will be required > (reconcat .flags from internal list structure) so I decided to omit it. > > > --- > src/fw_printenv.c | 16 +++-- > src/uboot_env.c | 176 ++++++++++++++++++++++++++++++---------------- > 2 files changed, 125 insertions(+), 67 deletions(-) > > diff --git a/src/fw_printenv.c b/src/fw_printenv.c > index 18887f9..8fd1f55 100644 > --- a/src/fw_printenv.c > +++ b/src/fw_printenv.c > @@ -137,17 +137,21 @@ int main (int argc, char **argv) { > if (!argc) { > tmp = NULL; > while ((tmp = libuboot_iterator(ctx, tmp)) != NULL) { > - name = libuboot_getname(tmp); > value = libuboot_getvalue(tmp); > - fprintf(stdout, "%s=%s\n", name, value); > + if (value) { > + name = libuboot_getname(tmp); > + fprintf(stdout, "%s=%s\n", name, value); > + } > } > } else { > for (i = 0; i < argc; i++) { > value = libuboot_get_env(ctx, argv[i]); > - if (noheader) > - fprintf(stdout, "%s\n", value ? value : ""); > - else > - fprintf(stdout, "%s=%s\n", argv[i], value ? value : > ""); > + if (value) { > + if (noheader) > + fprintf(stdout, "%s\n", value ? value : ""); > + else > + fprintf(stdout, "%s=%s\n", argv[i], value ? value > : ""); > + } > } > } > } else { /* setenv branch */ > diff --git a/src/uboot_env.c b/src/uboot_env.c > index 09d210d..bf60635 100644 > --- a/src/uboot_env.c > +++ b/src/uboot_env.c > @@ -735,6 +735,9 @@ int libuboot_env_store(struct uboot_ctx *ctx) > uint8_t offsetdata; > int ret; > int copy; > +#if !defined(NDEBUG) > + char * flagsbuf; > +#endif > > /* > * Allocate the bigger of the case > @@ -752,21 +755,27 @@ int libuboot_env_store(struct uboot_ctx *ctx) > > buf = data; > LIST_FOREACH(entry, &ctx->varlist, next) { > - size = (ctx->size - offsetdata) - (buf - data); > - if ((strlen(entry->name) + strlen(entry->value) + 2) > size) > - return -ENOMEM; > + if (entry->value) { > + /* only save variables which have a value (do exist yet) .. */ > + size = (ctx->size - offsetdata) - (buf - data); > + if ((strlen(entry->name) + strlen(entry->value) + 2) > size) > + return -ENOMEM; > > + buf += snprintf(buf, size, "%s=%s", entry->name, > entry->value); > + buf++; > + } > + /* .. but include non yet existing vars into .flags string */ > if (entry->type || entry->access) > saveflags = true; > - > - buf += snprintf(buf, size, "%s=%s", entry->name, entry->value); > - buf++; > } > > /* > * Now save the .flags > */ > if (saveflags) { > +#if !defined(NDEBUG) > + flagsbuf = buf; > +#endif > bool first = true; > size = (ctx->size - offsetdata) - (buf - data); > buf += snprintf(buf, size, ".flags="); > @@ -780,10 +789,15 @@ int libuboot_env_store(struct uboot_ctx *ctx) > attr_tostring(entry->type), > access_tostring(entry->access)); > } > + first = false; > } > } > *buf++ = '\0'; > > +#if !defined(NDEBUG) > + fprintf(stdout, "Saved environment FLAGS %s\n", flagsbuf); > +#endif > + > if (ctx->redundant) { > unsigned char flags = ctx->envdevs[ctx->current].flags; > switch(ctx->envdevs[ctx->current].flagstype) { > @@ -817,18 +831,19 @@ int libuboot_env_store(struct uboot_ctx *ctx) > return ret; > } > > -static void libuboot_parseflags(struct uboot_ctx *ctx, char* flagsvar) > +static void libuboot_parseflags(struct uboot_ctx *ctx, const char* flags) > { > char *pvar; > char *pval; > struct var_entry *entry; > + char *flagsvar = strdup(flags); > > #if !defined(NDEBUG) > - fprintf(stdout, "Environment FLAGS %s\n", flagsvar); > + fprintf(stdout, "Parsed environment FLAGS %s\n", flags); > #endif > pvar = flagsvar; > > - while (*pvar && (pvar - flagsvar) < strlen(flagsvar)) { > + while (pvar && ((pvar - flagsvar) < strlen(flags))) { > char *pnext; > pval = strchr(pvar, ':'); > if (!pval) > @@ -841,47 +856,56 @@ static void libuboot_parseflags(struct uboot_ctx > *ctx, char* flagsvar) > else > *pnext++ = '\0'; > > +#if !defined(NDEBUG) > + fprintf(stdout, "Parsed var %s:%s\n", pvar, pval); > +#endif > entry = __libuboot_get_env(&ctx->varlist, pvar); > - if (entry) { > - for (int i = 0; i < strlen(pval); i++) { > - switch (pval[i]) { > - case 's': > - entry->type = TYPE_ATTR_STRING; > - break; > - case 'd': > - entry->type = TYPE_ATTR_DECIMAL; > - break; > - case 'x': > - entry->type = TYPE_ATTR_HEX; > - break; > - case 'b': > - entry->type = TYPE_ATTR_BOOL; > - break; > - case 'i': > - entry->type = TYPE_ATTR_IP; > - break; > - case 'm': > - entry->type = TYPE_ATTR_MAC; > - break; > - case 'a': > - entry->access = ACCESS_ATTR_ANY; > - break; > - case 'r': > - entry->access = ACCESS_ATTR_READ_ONLY; > - break; > - case 'o': > - entry->access = ACCESS_ATTR_WRITE_ONCE; > - break; > - case 'c': > - entry->access = ACCESS_ATTR_CHANGE_DEFAULT; > - break; > - default: /* ignore it */ > - break; > - } > + if (!entry) { > + /* create non-valued entry to conserve its flags */ > + libuboot_set_env(ctx, pvar, NULL); > + entry = __libuboot_get_env(&ctx->varlist, pvar); > + } > + > + for (int i = 0; i < strlen(pval); i++) { > + switch (pval[i]) { > + case 's': > + entry->type = TYPE_ATTR_STRING; > + break; > + case 'd': > + entry->type = TYPE_ATTR_DECIMAL; > + break; > + case 'x': > + entry->type = TYPE_ATTR_HEX; > + break; > + case 'b': > + entry->type = TYPE_ATTR_BOOL; > + break; > + case 'i': > + entry->type = TYPE_ATTR_IP; > + break; > + case 'm': > + entry->type = TYPE_ATTR_MAC; > + break; > + case 'a': > + entry->access = ACCESS_ATTR_ANY; > + break; > + case 'r': > + entry->access = ACCESS_ATTR_READ_ONLY; > + break; > + case 'o': > + entry->access = ACCESS_ATTR_WRITE_ONCE; > + break; > + case 'c': > + entry->access = ACCESS_ATTR_CHANGE_DEFAULT; > + break; > + default: /* ignore it */ > + break; > } > } > + > pvar = pnext; > } > + free(flagsvar); > } > > static int libuboot_load(struct uboot_ctx *ctx) > @@ -1061,10 +1085,10 @@ int libuboot_load_file(struct uboot_ctx *ctx, > const char *filename) > > name = buf; > > - if (!strcmp(name, ".flags")) > - flagsvar = strdup(value); > + if (!strcmp(name, ".flags")) > + flagsvar = strdup(value); > else > - libuboot_set_env(ctx, name, value); > + libuboot_set_env(ctx, name, value); > } > > /* > @@ -1158,12 +1182,19 @@ static bool libuboot_validate_flags(struct > var_entry *entry, const char *value) > ok_access = true; > break; > case ACCESS_ATTR_READ_ONLY: > - case ACCESS_ATTR_WRITE_ONCE: > ok_access = false; > break; > + case ACCESS_ATTR_WRITE_ONCE: > + if (entry->value) { > + ok_access = false; > + } > + break; > case ACCESS_ATTR_CHANGE_DEFAULT: > break; > } > +#if !defined(NDEBUG) > + fprintf(stdout, "Access flag validation %s\n", ok_access ? "success." > : "failed."); > +#endif > > if (!ok_access) > return false; > @@ -1201,6 +1232,10 @@ static bool libuboot_validate_flags(struct > var_entry *entry, const char *value) > ok_type = false; > break; > } > +#if !defined(NDEBUG) > + fprintf(stdout, "Type flag validation %s\n", ok_type ? "success." : > "failed."); > +#endif > + > return ok_type; > } > > @@ -1209,13 +1244,30 @@ int libuboot_set_env(struct uboot_ctx *ctx, const > char *varname, const char *val > struct var_entry *entry, *elm, *lastentry; > struct vars *envs = &ctx->varlist; > > + /* update all .flags attributes in list with new value */ > + if (!strcmp(varname, ".flags")) { > + LIST_FOREACH(entry, &ctx->varlist, next) { > + entry->type = TYPE_ATTR_STRING; > + entry->access = ACCESS_ATTR_ANY; > + } > + if (value) > + libuboot_parseflags(ctx, strdup(value)); > + > + return 0; > + } > + > entry = __libuboot_get_env(envs, varname); > if (entry) { > if (libuboot_validate_flags(entry, value)) { > - if (!value) { > - free_var_entry(envs, entry); > - } else { > - free(entry->value); > + if (entry->value) { > + if (!value) { > + free_var_entry(envs, entry); > + } else { > + free(entry->value); > + entry->value = strdup(value); > + } > + } > + else if (value) { > entry->value = strdup(value); > } > return 0; > @@ -1224,9 +1276,6 @@ int libuboot_set_env(struct uboot_ctx *ctx, const > char *varname, const char *val > } > } > > - if (!value) > - return 0; > - > entry = (struct var_entry *)calloc(1, sizeof(*entry)); > if (!entry) > return -ENOMEM; > @@ -1235,11 +1284,16 @@ int libuboot_set_env(struct uboot_ctx *ctx, const > char *varname, const char *val > free(entry); > return -ENOMEM; > } > - entry->value = strdup(value); > - if (!entry->value) { > - free(entry->name); > - free(entry); > - return -ENOMEM; > + if (!value) { > + entry->value = NULL; > + } > + else { > + entry->value = strdup(value); > + if (!entry->value) { > + free(entry->name); > + free(entry); > + return -ENOMEM; > + } > } > > lastentry = NULL; > -- > 2.17.1 > > >
Hi Uwe, On 11.03.20 15:36, Uwe Schuster wrote: > Hi Stefano, > > can this patch be applied ? No. > We are using it in our products since a > while, but would like to have it part of the community version. Or is > there something else you need to be prepared? > A patch to be considered must follow some rules. Please check http://sbabic.github.io/swupdate/contributing.html. You should add your Signed-off-by to your patches and send them as plain text, so that it can automatically listed on http://patchwork.ozlabs.org/project/swupdate/list/, and then applied. It is like you have copy&paste the code, and this does not allow to apply the patch without rewriting the code. Use git "format-patch" and "git send-email" - this guarantees that the mail client won't modify them. Best regards, Stefano Babic > Uwe > > Am Freitag, 2. August 2019 13:49:44 UTC+2 schrieb Uwe Schuster: > > This tries to fix the problem with variable access or type flags > which are part of the .flags variable but do not exist as a env > variable as reported by me in > https://groups.google.com/d/msg/swupdate/ShdD10dl7Pc/ULUYSpFDCAAJ > <https://groups.google.com/d/msg/swupdate/ShdD10dl7Pc/ULUYSpFDCAAJ>: > > 1. The write once (o) access atrribute flag is not properly > handled if the variable referenced by the .flags value string > does not exist yet (as it would be the case with write once > variables before they are first written or set). Instead this > variables flags gets removed from .flags value and attributes > are lost. > This behavior also applies to all other variables which do not > exist yet, but in case of write once variables I'd consider it a > normal use case. > > > I had to choose an implementation and I chose to put such uncreated > variables into the metadata database as normal variables but with > NULL value assigned. > > The patch also fixes a problem with the existing parsing function. > It failed to handle .flags value strings with more then one variable > (comma separated). > > The only known problem what remains is with fw_printenv function. It > does currently not print out the .flags variable (as normal u-boot > would). To fix that more changes to the printenv implementation will > be required (reconcat .flags from internal list structure) so I > decided to omit it. > > > --- > src/fw_printenv.c | 16 +++-- > src/uboot_env.c | 176 ++++++++++++++++++++++++++++++---------------- > 2 files changed, 125 insertions(+), 67 deletions(-) > > diff --git a/src/fw_printenv.c b/src/fw_printenv.c > index 18887f9..8fd1f55 100644 > --- a/src/fw_printenv.c > +++ b/src/fw_printenv.c > @@ -137,17 +137,21 @@ int main (int argc, char **argv) { > if (!argc) { > tmp = NULL; > while ((tmp = libuboot_iterator(ctx, tmp)) != NULL) { > - name = libuboot_getname(tmp); > value = libuboot_getvalue(tmp); > - fprintf(stdout, "%s=%s\n", name, value); > + if (value) { > + name = libuboot_getname(tmp); > + fprintf(stdout, "%s=%s\n", name, value); > + } > } > } else { > for (i = 0; i < argc; i++) { > value = libuboot_get_env(ctx, argv[i]); > - if (noheader) > - fprintf(stdout, "%s\n", value ? value : ""); > - else > - fprintf(stdout, "%s=%s\n", argv[i], value ? > value : ""); > + if (value) { > + if (noheader) > + fprintf(stdout, "%s\n", value ? value : ""); > + else > + fprintf(stdout, "%s=%s\n", argv[i], value ? > value : ""); > + } > } > } > } else { /* setenv branch */ > diff --git a/src/uboot_env.c b/src/uboot_env.c > index 09d210d..bf60635 100644 > --- a/src/uboot_env.c > +++ b/src/uboot_env.c > @@ -735,6 +735,9 @@ int libuboot_env_store(struct uboot_ctx *ctx) > uint8_t offsetdata; > int ret; > int copy; > +#if !defined(NDEBUG) > + char * flagsbuf; > +#endif > > /* > * Allocate the bigger of the case > @@ -752,21 +755,27 @@ int libuboot_env_store(struct uboot_ctx *ctx) > > buf = data; > LIST_FOREACH(entry, &ctx->varlist, next) { > - size = (ctx->size - offsetdata) - (buf - data); > - if ((strlen(entry->name) + strlen(entry->value) + 2) > size) > - return -ENOMEM; > + if (entry->value) { > + /* only save variables which have a value (do exist > yet) .. */ > + size = (ctx->size - offsetdata) - (buf - data); > + if ((strlen(entry->name) + strlen(entry->value) + 2) > > size) > + return -ENOMEM; > > + buf += snprintf(buf, size, "%s=%s", entry->name, > entry->value); > + buf++; > + } > + /* .. but include non yet existing vars into .flags string */ > if (entry->type || entry->access) > saveflags = true; > - > - buf += snprintf(buf, size, "%s=%s", entry->name, > entry->value); > - buf++; > } > > /* > * Now save the .flags > */ > if (saveflags) { > +#if !defined(NDEBUG) > + flagsbuf = buf; > +#endif > bool first = true; > size = (ctx->size - offsetdata) - (buf - data); > buf += snprintf(buf, size, ".flags="); > @@ -780,10 +789,15 @@ int libuboot_env_store(struct uboot_ctx *ctx) > attr_tostring(entry->type), > access_tostring(entry->access)); > } > + first = false; > } > } > *buf++ = '\0'; > > +#if !defined(NDEBUG) > + fprintf(stdout, "Saved environment FLAGS %s\n", flagsbuf); > +#endif > + > if (ctx->redundant) { > unsigned char flags = ctx->envdevs[ctx->current].flags; > switch(ctx->envdevs[ctx->current].flagstype) { > @@ -817,18 +831,19 @@ int libuboot_env_store(struct uboot_ctx *ctx) > return ret; > } > > -static void libuboot_parseflags(struct uboot_ctx *ctx, char* flagsvar) > +static void libuboot_parseflags(struct uboot_ctx *ctx, const char* > flags) > { > char *pvar; > char *pval; > struct var_entry *entry; > + char *flagsvar = strdup(flags); > > #if !defined(NDEBUG) > - fprintf(stdout, "Environment FLAGS %s\n", flagsvar); > + fprintf(stdout, "Parsed environment FLAGS %s\n", flags); > #endif > pvar = flagsvar; > > - while (*pvar && (pvar - flagsvar) < strlen(flagsvar)) { > + while (pvar && ((pvar - flagsvar) < strlen(flags))) { > char *pnext; > pval = strchr(pvar, ':'); > if (!pval) > @@ -841,47 +856,56 @@ static void libuboot_parseflags(struct > uboot_ctx *ctx, char* flagsvar) > else > *pnext++ = '\0'; > > +#if !defined(NDEBUG) > + fprintf(stdout, "Parsed var %s:%s\n", pvar, pval); > +#endif > entry = __libuboot_get_env(&ctx->varlist, pvar); > - if (entry) { > - for (int i = 0; i < strlen(pval); i++) { > - switch (pval[i]) { > - case 's': > - entry->type = TYPE_ATTR_STRING; > - break; > - case 'd': > - entry->type = TYPE_ATTR_DECIMAL; > - break; > - case 'x': > - entry->type = TYPE_ATTR_HEX; > - break; > - case 'b': > - entry->type = TYPE_ATTR_BOOL; > - break; > - case 'i': > - entry->type = TYPE_ATTR_IP; > - break; > - case 'm': > - entry->type = TYPE_ATTR_MAC; > - break; > - case 'a': > - entry->access = ACCESS_ATTR_ANY; > - break; > - case 'r': > - entry->access = ACCESS_ATTR_READ_ONLY; > - break; > - case 'o': > - entry->access = ACCESS_ATTR_WRITE_ONCE; > - break; > - case 'c': > - entry->access = ACCESS_ATTR_CHANGE_DEFAULT; > - break; > - default: /* ignore it */ > - break; > - } > + if (!entry) { > + /* create non-valued entry to conserve its flags */ > + libuboot_set_env(ctx, pvar, NULL); > + entry = __libuboot_get_env(&ctx->varlist, pvar); > + } > + > + for (int i = 0; i < strlen(pval); i++) { > + switch (pval[i]) { > + case 's': > + entry->type = TYPE_ATTR_STRING; > + break; > + case 'd': > + entry->type = TYPE_ATTR_DECIMAL; > + break; > + case 'x': > + entry->type = TYPE_ATTR_HEX; > + break; > + case 'b': > + entry->type = TYPE_ATTR_BOOL; > + break; > + case 'i': > + entry->type = TYPE_ATTR_IP; > + break; > + case 'm': > + entry->type = TYPE_ATTR_MAC; > + break; > + case 'a': > + entry->access = ACCESS_ATTR_ANY; > + break; > + case 'r': > + entry->access = ACCESS_ATTR_READ_ONLY; > + break; > + case 'o': > + entry->access = ACCESS_ATTR_WRITE_ONCE; > + break; > + case 'c': > + entry->access = ACCESS_ATTR_CHANGE_DEFAULT; > + break; > + default: /* ignore it */ > + break; > } > } > + > pvar = pnext; > } > + free(flagsvar); > } > > static int libuboot_load(struct uboot_ctx *ctx) > @@ -1061,10 +1085,10 @@ int libuboot_load_file(struct uboot_ctx > *ctx, const char *filename) > > name = buf; > > - if (!strcmp(name, ".flags")) > - flagsvar = strdup(value); > + if (!strcmp(name, ".flags")) > + flagsvar = strdup(value); > else > - libuboot_set_env(ctx, name, value); > + libuboot_set_env(ctx, name, value); > } > > /* > @@ -1158,12 +1182,19 @@ static bool libuboot_validate_flags(struct > var_entry *entry, const char *value) > ok_access = true; > break; > case ACCESS_ATTR_READ_ONLY: > - case ACCESS_ATTR_WRITE_ONCE: > ok_access = false; > break; > + case ACCESS_ATTR_WRITE_ONCE: > + if (entry->value) { > + ok_access = false; > + } > + break; > case ACCESS_ATTR_CHANGE_DEFAULT: > break; > } > +#if !defined(NDEBUG) > + fprintf(stdout, "Access flag validation %s\n", ok_access ? > "success." : "failed."); > +#endif > > if (!ok_access) > return false; > @@ -1201,6 +1232,10 @@ static bool libuboot_validate_flags(struct > var_entry *entry, const char *value) > ok_type = false; > break; > } > +#if !defined(NDEBUG) > + fprintf(stdout, "Type flag validation %s\n", ok_type ? > "success." : "failed."); > +#endif > + > return ok_type; > } > > @@ -1209,13 +1244,30 @@ int libuboot_set_env(struct uboot_ctx *ctx, > const char *varname, const char *val > struct var_entry *entry, *elm, *lastentry; > struct vars *envs = &ctx->varlist; > > + /* update all .flags attributes in list with new value */ > + if (!strcmp(varname, ".flags")) { > + LIST_FOREACH(entry, &ctx->varlist, next) { > + entry->type = TYPE_ATTR_STRING; > + entry->access = ACCESS_ATTR_ANY; > + } > + if (value) > + libuboot_parseflags(ctx, strdup(value)); > + > + return 0; > + } > + > entry = __libuboot_get_env(envs, varname); > if (entry) { > if (libuboot_validate_flags(entry, value)) { > - if (!value) { > - free_var_entry(envs, entry); > - } else { > - free(entry->value); > + if (entry->value) { > + if (!value) { > + free_var_entry(envs, entry); > + } else { > + free(entry->value); > + entry->value = strdup(value); > + } > + } > + else if (value) { > entry->value = strdup(value); > } > return 0; > @@ -1224,9 +1276,6 @@ int libuboot_set_env(struct uboot_ctx *ctx, > const char *varname, const char *val > } > } > > - if (!value) > - return 0; > - > entry = (struct var_entry *)calloc(1, sizeof(*entry)); > if (!entry) > return -ENOMEM; > @@ -1235,11 +1284,16 @@ int libuboot_set_env(struct uboot_ctx *ctx, > const char *varname, const char *val > free(entry); > return -ENOMEM; > } > - entry->value = strdup(value); > - if (!entry->value) { > - free(entry->name); > - free(entry); > - return -ENOMEM; > + if (!value) { > + entry->value = NULL; > + } > + else { > + entry->value = strdup(value); > + if (!entry->value) { > + free(entry->name); > + free(entry); > + return -ENOMEM; > + } > } > > lastentry = NULL; > -- > 2.17.1 > > > -- > You received this message because you are subscribed to the Google > Groups "swupdate" group. > To unsubscribe from this group and stop receiving emails from it, send > an email to swupdate+unsubscribe@googlegroups.com > <mailto:swupdate+unsubscribe@googlegroups.com>. > To view this discussion on the web visit > https://groups.google.com/d/msgid/swupdate/f8bd90e8-8d25-4e5b-bcd8-9bae98229825%40googlegroups.com > <https://groups.google.com/d/msgid/swupdate/f8bd90e8-8d25-4e5b-bcd8-9bae98229825%40googlegroups.com?utm_medium=email&utm_source=footer>.
diff --git a/src/fw_printenv.c b/src/fw_printenv.c index 18887f9..8fd1f55 100644 --- a/src/fw_printenv.c +++ b/src/fw_printenv.c @@ -137,17 +137,21 @@ int main (int argc, char **argv) { if (!argc) { tmp = NULL; while ((tmp = libuboot_iterator(ctx, tmp)) != NULL) { - name = libuboot_getname(tmp); value = libuboot_getvalue(tmp); - fprintf(stdout, "%s=%s\n", name, value); + if (value) { + name = libuboot_getname(tmp); + fprintf(stdout, "%s=%s\n", name, value); + } } } else { for (i = 0; i < argc; i++) { value = libuboot_get_env(ctx, argv[i]); - if (noheader) - fprintf(stdout, "%s\n", value ? value : ""); - else - fprintf(stdout, "%s=%s\n", argv[i], value ? value : ""); + if (value) { + if (noheader) + fprintf(stdout, "%s\n", value ? value : ""); + else + fprintf(stdout, "%s=%s\n", argv[i], value ? value : ""); + } } } } else { /* setenv branch */ diff --git a/src/uboot_env.c b/src/uboot_env.c index 09d210d..bf60635 100644 --- a/src/uboot_env.c +++ b/src/uboot_env.c @@ -735,6 +735,9 @@ int libuboot_env_store(struct uboot_ctx *ctx) uint8_t offsetdata; int ret; int copy; +#if !defined(NDEBUG) + char * flagsbuf; +#endif /* * Allocate the bigger of the case @@ -752,21 +755,27 @@ int libuboot_env_store(struct uboot_ctx *ctx) buf = data; LIST_FOREACH(entry, &ctx->varlist, next) { - size = (ctx->size - offsetdata) - (buf - data); - if ((strlen(entry->name) + strlen(entry->value) + 2) > size) - return -ENOMEM; + if (entry->value) { + /* only save variables which have a value (do exist yet) .. */ + size = (ctx->size - offsetdata) - (buf - data); + if ((strlen(entry->name) + strlen(entry->value) + 2) > size) + return -ENOMEM; + buf += snprintf(buf, size, "%s=%s", entry->name, entry->value); + buf++; + } + /* .. but include non yet existing vars into .flags string */ if (entry->type || entry->access) saveflags = true; - - buf += snprintf(buf, size, "%s=%s", entry->name, entry->value); - buf++; } /* * Now save the .flags */ if (saveflags) { +#if !defined(NDEBUG) + flagsbuf = buf; +#endif bool first = true; size = (ctx->size - offsetdata) - (buf - data);