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

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

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.

Thanks,
Qu
>