Re: [PATCh v2 5/9] btrfs: tree-checker: Verify dev item

[Johannes Thumshirn <jthumshirn@suse.de>]

On 20/03/2019 07:37, Qu Wenruo wrote:
[...]

> +static int check_dev_item(struct btrfs_fs_info *fs_info,
> +			  struct extent_buffer *leaf,
> +			  struct btrfs_key *key, int slot)
> +{
> +	struct btrfs_dev_item *ditem;
> +	u64 max_devid = max(BTRFS_MAX_DEVS(fs_info), BTRFS_MAX_DEVS_SYS_CHUNK);
> +
> +	if (key->objectid != BTRFS_DEV_ITEMS_OBJECTID) {
> +		dev_item_err(fs_info, leaf, slot,
> +			     "invalid objectid: has=%llu expect=%llu",
> +			     key->objectid, BTRFS_DEV_ITEMS_OBJECTID);
> +		goto error;
> +	}
> +	if (key->offset > max_devid) {
> +		dev_item_err(fs_info, leaf, slot,
> +			     "invalid devid: has=%llu expect=[0, %llu]",
> +			     key->offset, max_devid);
> +		goto error;
> +	}
> +	ditem = btrfs_item_ptr(leaf, slot, struct btrfs_dev_item);
> +	if (btrfs_device_id(leaf, ditem) != key->offset) {
> +		dev_item_err(fs_info, leaf, slot,
> +			     "devid mismatch: key has=%llu item has=%llu",
> +			     key->offset, btrfs_device_id(leaf, ditem));
> +		goto error;
> +	}
> +
> +	/*
> +	 * Since btrfs device add doesn't check device size at all, we could
> +	 * have device item whose size is smaller than 1M which is useless, but
> +	 * still valid.
> +	 * So here we can only check the obviously wrong case.
> +	 */
> +	if (btrfs_device_total_bytes(leaf, ditem) == 0) {
> +		dev_item_err(fs_info, leaf, slot,
> +			     "invalid total bytes: have 0");
> +		goto error;
> +	}
> +	if (btrfs_device_bytes_used(leaf, ditem) >
> +	    btrfs_device_total_bytes(leaf, ditem)) {
> +		dev_item_err(fs_info, leaf, slot,
> +			     "invalid bytes used: have %llu expect [0, %llu]",
> +			     btrfs_device_bytes_used(leaf, ditem),
> +			     btrfs_device_total_bytes(leaf, ditem));
> +		goto error;
> +	}
> +	/*
> +	 * Remaining members like io_align/type/gen/dev_group aren't really
> +	 * utilized.
> +	 * Skip them to make later usage of them easier.
> +	 */
> +	return 0;
> +error:
> +	return -EUCLEAN;
> +}
> +

Why aren't you directly returning -EUCLEAN instead of the gotos? There's
no cleanup pending so the additional jump label is unnecessary.



-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850