On 2019/3/20 下午7:51, Johannes Thumshirn wrote:
> 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.

Just a coding preference.

Will it impact the performance or compiler is clever enough to change
the goto line to return -EUCLEAN?

Thanks,
Qu

Reply via email to