On Thu, Oct 02, 2025 at 05:47:59PM +0200, Daniel Kiper wrote: > 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()/
OK. > > In general please mark function names in the commit messages and > comments with "()". Ok, will take another look to make sure they are all corrected. > > > 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; OK, will fix next version. > > > +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... OK, will fix next version. > > > + 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? OK, I don't like this name I came up with either. I'll switch to "envblk_on_block" you proposed later in the series - much better. > > > 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... OK, will fix next version. Thanks, Michael > > Daniel _______________________________________________ Grub-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/grub-devel
