Re: [PATCH 1/3] btrfs: volumes: Refactor device holes gathering into a separate function

[Johannes Thumshirn <jthumshirn@suse.de>]

On 07/11/2019 10:33, Qu Wenruo wrote:
> 
> 
> On 2019/11/7 下午5:20, Johannes Thumshirn wrote:
>> On 07/11/2019 07:27, Qu Wenruo wrote:
>>> +static int gather_dev_holes(struct btrfs_fs_info *info,
>>> +			    struct btrfs_device_info *devices_info,
>>> +			    int *index, struct list_head *list,
>>> +			    int max_nr_devs, u64 stripe_size, int dev_stripes)
>>> +{
>>
>> Hi Qu,
>>
>> What do you think of
>>
>> static int gather_dev_holes(struct btrfs_fs_info *info,
>> 			    int *index,	u64 stripe_size,
>>                             int dev_stripes)
>> {
>> 	struct btrfs_fs_devices *fs_devices = info->fs_devices;
>> 	struct btrfs_device *device;
>> 	int ret;
>> 	int ndevs = 0;
>>
>> 	list_for_each_entry(device, &fs_devices->alloc_list,
>> 			    dev_alloc_list) {
>>
>> [...]
>>
>>
>> btrfs_device_info can be derived from btrfs_fs_info, and *list and
>> max_nr_devs can be derived from btrfs_device_info.
>>
>> This reduces the number of arguments by 3 and we don't need to pass that
>> odd 'struct list_head *list' which isn't really clear from the type what
>> it is referring to.
> 
> The objective of this refactor is to accept different list, not only
> fs_devices->alloc_list, but also later fs_devices->missing_list.
> 
> So I'm afraid it's not that easy to reduce the number of paramters.


hmm this makes the api of gather_dev_holes() even more intransparent.

Looking at patch 3/3 I see why you're doing it though.

-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850