On 14.12.21 г. 16:45, Josef Bacik wrote:

On Tue, Dec 14, 2021 at 03:39:39PM +0200, Nikolay Borisov wrote:

quoted

The purpose of this function is to unlock all nodes in a btrfs path
which are above 'lowest_unlock' and whose slot used is different than 0.
As such it used slightly awkward structure of 'if' as well as somewhat
cryptic "no_skip" control variable which denotes whether we should
check the current level of skipiability or no.

This patch does the following (cosmetic) refactorings:

* Renames 'no_skip' to 'check_skip' and makes it a boolean. This
variable controls whether we are below the lowest_unlock/skip_level
levels.

* Consolidates the 2 conditions which warrant checking whether the
current level should be skipped under 1 common if (check_skip) branch,
this increase indentation level but is not critical.

* Consolidates the 'skip_level < i && i >= lowest_unlock' and
'i >= lowest_unlock && i > skip_level' condition into a common branch
since those are identical.

* Eliminates the local extent_buffer variable as in this case it doesn't
bring anything to function readability.

Signed-off-by: Nikolay Borisov <redacted>

This was weirdly difficult to review in both diff and vimdiff, had to look at
the resulting code to see how it worked out.

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Yeah, sorry about that but the function has a bunch of IF's ... I could
have probably broken it into 3 patches but each separate refactoring is
really small.

Thanks,

Josef