Re: [PATCH 2/7] Add helper to check the btrfs header.

[Daniel Kiper]

On Tue, Apr 24, 2018 at 09:13:11PM +0200, Goffredo Baroncelli wrote:
> This helper was used in few places to help the debugging. As conservative
> approach, in case of error it is only logged.

Could you explain in the commit message why we are so conservative here?

> Signed-off-by: Goffredo Baroncelli <address@hidden>
> ---
>  grub-core/fs/btrfs.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> index b0032ea46..01a1fc7a1 100644
> --- a/grub-core/fs/btrfs.c
> +++ b/grub-core/fs/btrfs.c
> @@ -77,7 +77,8 @@ struct btrfs_header
>  {
>    grub_btrfs_checksum_t checksum;
>    grub_btrfs_uuid_t uuid;
> -  grub_uint8_t dummy[0x30];
> +  grub_uint64_t bytenr;
> +  grub_uint8_t dummy[0x28];
>    grub_uint32_t nitems;
>    grub_uint8_t level;
>  } GRUB_PACKED;
> @@ -286,6 +287,23 @@ free_iterator (struct grub_btrfs_leaf_descriptor *desc)
>    grub_free (desc->data);
>  }
>
> +static grub_err_t
> +check_btrfs_header (struct grub_btrfs_data *data, struct btrfs_header 
> *header,
> +                    grub_disk_addr_t addr)
> +{
> +  if (grub_le_to_cpu64 (header->bytenr) != addr)
> +    {
> +      grub_dprintf ("btrfs", "btrfs_header.bytenr is not addr\n");
> +      return grub_error (GRUB_ERR_BAD_FS, "header bytenr is not addr");

s/is not addr/is not equal node addr/?

> +    }
> +  if (grub_memcmp (data->sblock.uuid, header->uuid, 
> sizeof(grub_btrfs_uuid_t)))
> +    {
> +      grub_dprintf ("btrfs", "btrfs_header.uuid doesn't match\n");
> +      return grub_error (GRUB_ERR_BAD_FS, "header uuid doesn't match");

s/doesn't match/doesn't match sblock uuid/?

Daniel