Re: [PATCH v2 3/9] util/grub-editenv: add fs_envblk write helper

[Avnish Chouhan <avnish@linux.ibm.com>]

On 2025-09-15 14:39, grub-devel-request@gnu.org wrote:
  > Message: 4
> Date: Mon, 15 Sep 2025 17:08:42 +0800
> From: Michael Chang <mchang@suse.com>
> To: The development of GNU GRUB <grub-devel@gnu.org>
> Cc: Neal Gompa <ngompa13@gmail.com>,	Marta Lewandowska
> 	<mlewando@redhat.com>
> Subject: [PATCH v2 3/9] util/grub-editenv: add fs_envblk write helper
> Message-ID: <20250915090848.131937-4-mchang@suse.com>
> 
> This patch adds the function fs_envblk_write to update the reserved
> environment block on disk. The helper takes an in memory envblk buffer
> and writes it back to the device at the location defined by the
> fs_envblk specification. It performs size checks and uses file sync to
> ensure that the updated data is flushed.
> 
> The helper is also added into the fs_envblk ops table, together with 
> the
> open helper from the previous patch. With this change the basic input
> and output path for an external environment block is complete. The
> choice of which variables should be written externally will be handled
> by later patches.
> 
> Signed-off-by: Michael Chang <mchang@suse.com>
> ---
>  util/grub-editenv.c | 38 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/util/grub-editenv.c b/util/grub-editenv.c
> index 4e5dffa86..26a81d2d0 100644
> --- a/util/grub-editenv.c
> +++ b/util/grub-editenv.c
> @@ -131,9 +131,11 @@ struct fs_envblk_spec {
>  typedef struct fs_envblk_spec *fs_envblk_spec_t;
> 
>  static grub_envblk_t fs_envblk_open (grub_envblk_t envblk);
> +static void fs_envblk_write (grub_envblk_t envblk);
> 
>  struct fs_envblk_ops {
>    grub_envblk_t (*open) (grub_envblk_t);
> +  void (*write) (grub_envblk_t);
>  };
> 
>  struct fs_envblk {
> @@ -144,7 +146,8 @@ struct fs_envblk {
>  typedef struct fs_envblk *fs_envblk_t;
> 
>  static struct fs_envblk_ops fs_envblk_ops = {
> -  .open = fs_envblk_open
> +  .open = fs_envblk_open,
> +  .write = fs_envblk_write
>  };
> 
>  static struct fs_envblk_spec fs_envblk_spec[] = {
> @@ -358,6 +361,39 @@ write_envblk (const char *name, grub_envblk_t 
> envblk)
>    fclose (fp);
>  }
> 
> +static void
> +fs_envblk_write (grub_envblk_t envblk)
> +{
> +  FILE *fp;
> +  const char *device;
> +  off_t offset;
> +  size_t size;
> +
> +  if (envblk == NULL)
> +    return;
> +
> +  device = fs_envblk->dev;
> +  offset = fs_envblk->spec->offset;
> +  size = fs_envblk->spec->size;
> +
> +  if (grub_envblk_size (envblk) > size)
> +    grub_util_error ("%s", _("environment block too small"));
> +
> +  fp = grub_util_fopen (device, "r+b");
> +
> +  if (! fp)

Hi Michael,

(fp == NUL) would be better here.

> +    grub_util_error (_("cannot open `%s': %s"), device, strerror 
> (errno));
> +
> +  if (fseek (fp, offset, SEEK_SET) < 0)

fclose (fp) missing here!

> +    grub_util_error (_("cannot seek `%s': %s"), device, strerror 
> (errno));
> +
> +  if (fwrite (grub_envblk_buffer (envblk), 1, grub_envblk_size
> (envblk), fp) != grub_envblk_size (envblk))

fclose (fp) missing here!

Thank you!

Regards,
Avnish Chouhan

> +    grub_util_error (_("cannot write to `%s': %s"), device, strerror 
> (errno));
> +
> +  grub_util_file_sync (fp);
> +  fclose (fp);
> +}
> +
>  static void
>  set_variables (const char *name, int argc, char *argv[])
>  {
> --
> 2.51.0

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel