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