On 20.03.2018 11:53, Anand Jain wrote:
> %csum_type is being checked to check the number of csum types we support,
> which is 1 as of now. Instead just check if the type matches with the
> only type we support, that is BTRFS_CSUM_TYPE_CRC32. And further adds
> cleanup.
> 
> Signed-off-by: Anand Jain <anand.j...@oracle.com>
> ---
>  fs/btrfs/disk-io.c | 41 +++++++++++++++++++----------------------
>  1 file changed, 19 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 1657d6aa4fa6..582ed6af3c50 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -399,32 +399,29 @@ static int btrfs_check_super_csum(struct btrfs_fs_info 
> *fs_info,
>       struct btrfs_super_block *disk_sb =
>               (struct btrfs_super_block *)raw_disk_sb;
>       u16 csum_type = btrfs_super_csum_type(disk_sb);
> -     int ret = 0;
> -
> -     if (csum_type == BTRFS_CSUM_TYPE_CRC32) {
> -             u32 crc = ~(u32)0;
> -             char result[sizeof(crc)];
> -
> -             /*
> -              * The super_block structure does not span the whole
> -              * BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
> -              * is filled with zeros and is included in the checksum.
> -              */
> -             crc = btrfs_csum_data(raw_disk_sb + BTRFS_CSUM_SIZE,
> -                             crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
> -             btrfs_csum_final(crc, result);
> -
> -             if (memcmp(raw_disk_sb, result, sizeof(result)))
> -                     ret = 1;
> -     }
> +     u32 crc = ~(u32)0;
> +     char result[sizeof(crc)];
>  
> -     if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) {
> +     /* We support csum type crc32 only as of now */
> +     if (csum_type != BTRFS_CSUM_TYPE_CRC32) {
>               btrfs_err(fs_info, "unsupported checksum algorithm %u",
> -                             csum_type);
> -             ret = 1;
> +                       csum_type);
> +             return 1;

I think it would make sense to return EINVAL here. Then the sole caller
of this function (open_ctree) can use the code a bit more idiomatically:

ret = btrfs_check_super_csum()

if (ret < 0) {
handle failure
}

Right now we return 1 on both when the function fails due to invalid CRC
algorithm and if the checksum doesn't match. And we are going to print 2
things: unsupported checksum algorithm and the
btrfs_err(fs_info, "superblock checksum mismatch");    from open_ctree.

Let's implemented proper error returning strategy for this function.



>       }
>  
> -     return ret;
> +     /*
> +      * The super_block structure does not span the whole
> +      * BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
> +      * is filled with zeros and is included in the checksum.
> +      */
> +     crc = btrfs_csum_data(raw_disk_sb + BTRFS_CSUM_SIZE,
> +                           crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
> +     btrfs_csum_final(crc, result);
> +
> +     if (memcmp(raw_disk_sb, result, sizeof(result)))
> +             return 1;

Perhaps return EUCLEAN i.e something is corrupted hence the checksum
didn't pan out as we expected.

> +
> +     return 0;
>  }
>  
>  /*
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to