On 16.05.19 г. 11:47 ч., Johannes Thumshirn wrote:
> Now that we have already checked for a valid checksum type before calling
> btrfs_check_super_csum(), it can be simplified even further.
>
> While at it get rid of the implicit size assumption of the resulting
> checksum as well.
>
> Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>
> Reviewed-by: Nikolay Borisov <nbori...@suse.com>
>
> ---
> Changes to v1:
> - Check for disk_sb->csum instead of raw buffer (Nikolay)
> ---
> fs/btrfs/disk-io.c | 37 +++++++++++++------------------------
> 1 file changed, 13 insertions(+), 24 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 74937effaed4..edb8bc79b01b 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -375,33 +375,22 @@ static int btrfs_check_super_csum(struct btrfs_fs_info
> *fs_info,
> {
This function no longer requires the btrfs_fs_info argument so it should
be removed. While on the topic of refactoring this function - why not
change it's return type to bool since it can't return anything other
than 0/1 ?
nit: Will it make more sense if this function was named
btrfs_validate_super_csum ?
> 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 (!btrfs_supported_super_csum(disk_sb)) {
> - btrfs_err(fs_info, "unsupported checksum algorithm %u",
> - csum_type);
> - ret = 1;
> - }
> -
> - if (csum_type == BTRFS_CSUM_TYPE_CRC32) {
> - u32 crc = ~(u32)0;
> - char result[sizeof(crc)];
> + u32 crc = ~(u32)0;
> + char result[BTRFS_CSUM_SIZE];
>
> - /*
> - * 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);
> + /*
> + * 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;
> - }
> + if (memcmp(disk_sb->csum, result, btrfs_super_csum_size(disk_sb)))
> + return 1;
>
> - return ret;
> + return 0;
> }
>
> int btrfs_verify_level_key(struct extent_buffer *eb, int level,
>
