On Thu, Oct 02, 2025 at 02:46:17PM +0800, Michael Chang via Grub-devel wrote: > This patch changes set_variables so that it can use an external
s/set_variables/set_variables()/ In general please mark function names in the commit messages and comments with "()". > environment block when one is present. The variable next_entry is > written into the external block, env_block is treated as read only, and > all other variables are written into the normal file based envblk. > > A cleanup step is added to handle cases where GRUB at runtime writes > variables into the external block because file based updates are not > safe on a copy on write filesystem such as Btrfs. For example, the > savedefault command can update saved_entry, and on Btrfs GRUB will place > that update in the external block instead of the file envblk. If an > older copy remains in the external block, it would override the newer > value from the file envblk when GRUB first loads the file and then > applies the external block on top of it. To avoid this, whenever a > variable is updated in the file envblk, any same named key in the > external block is deleted. > > Signed-off-by: Michael Chang <[email protected]> > Reviewed-by: Neal Gompa <[email protected]> > --- > util/grub-editenv.c | 55 +++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 53 insertions(+), 2 deletions(-) > > diff --git a/util/grub-editenv.c b/util/grub-editenv.c > index 450bb00a5..efc6ec5f2 100644 > --- a/util/grub-editenv.c > +++ b/util/grub-editenv.c > @@ -394,12 +394,33 @@ fs_envblk_write (grub_envblk_t envblk) > fclose (fp); > } > > +struct var_lookup_ctx { > + const char *varname; > + bool found; > +}; typedef struct var_lookup_ctx var_lookup_ctx_t; > +static int > +var_lookup_iter (const char *varname, const char *value __attribute__ > ((unused)), void *hook_data) > +{ > + struct var_lookup_ctx *ctx = (struct var_lookup_ctx *) hook_data; Missing empty line... > + if (grub_strcmp (ctx->varname, varname) == 0) > + { > + ctx->found = true; > + return 1; > + } > + return 0; > +} > + > static void > set_variables (const char *name, int argc, char *argv[]) > { > grub_envblk_t envblk; > + grub_envblk_t envblk_fs = NULL; I am not happy with two almost the same names. Could you come up with more meaningful variable names here? > envblk = open_envblk_file (name); > + if (fs_envblk != NULL) > + envblk_fs = fs_envblk->ops->open (envblk); > + > while (argc) > { > char *p; > @@ -410,8 +431,32 @@ set_variables (const char *name, int argc, char *argv[]) > > *(p++) = 0; > > - if (! grub_envblk_set (envblk, argv[0], p)) > - grub_util_error ("%s", _("environment block too small")); > + if ((strcmp (argv[0], "next_entry") == 0) && envblk_fs) > + { > + if (! grub_envblk_set (envblk_fs, argv[0], p)) > + grub_util_error ("%s", _("environment block too small")); > + } > + else if (strcmp (argv[0], "env_block") == 0) > + { > + grub_util_warn (_("can't set env_block as it's read-only")); > + } Please drop redundant curly braces... Daniel _______________________________________________ Grub-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/grub-devel
