Re: [PATCH 1/3] btrfs: ctree: Reduce one indent level for btrfs_search_slot()

[Nikolay Borisov <nborisov@suse.com>]

On 10.09.19 г. 11:31 ч., Qu Wenruo wrote:
> 
> 
> On 2019/9/10 下午4:24, Nikolay Borisov wrote:
>>
>>
>> On 10.09.19 г. 10:40 ч., Qu Wenruo wrote:
>>> In btrfs_search_slot(), we something like:
>>>
>>> 	if (level != 0) {
>>> 		/* Do search inside tree nodes*/
>>> 	} else {
>>> 		/* Do search inside tree leaves */
>>> 		goto done;
>>> 	}
>>>
>>> This caused extra indent for tree node search code.
>>> Change it to something like:
>>>
>>> 	if (level == 0) {
>>> 		/* Do search inside tree leaves */
>>> 		goto done'
>>> 	}
>>> 	/* Do search inside tree nodes */
>>>
>>> So we have more space to maneuver our code, this is especially useful as
>>> the tree nodes search code is more complex than the leaves search code.
>>>
>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>
>> I actually thing this patch makes comprehending the function worse.
> 
> If the level == 0 lines is over 50 lines, maybe.
> 
> But it's just 22 lines.
>> Because the else is now somewhat implicit. E.g. one has to pay careful
>> attention to the contents inside the first if and especially the
>> unconditional 'goto done' to be able to understand the code after the
>> 'if' construct.
> 
> That's the same for the original code, you need to go a level upper to
> see we're in level > 0 branch.

But that's explicit with the 'if'

> 
> Thanks,
> Qu
>>
>