Re: [PATCH v2 09/13] btrfs: Simplify btrfs_check_super_csum() and get rid of size assumptions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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 <jthumshirn@xxxxxxx>
> Reviewed-by: Nikolay Borisov <nborisov@xxxxxxxx>
> 
> ---
> 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,
> 



[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux