On Thu, 11 May 2017 17:42:21 +0800 Leno Hou <leno...@gmail.com> wrote:
> This patch optimized the code by previously getpos function call.
> Therefore, It's takes the convenience to understand logic of code.
I would rewrite this changelog to read
: Rework the getpos() helper function and use it to remove various
: open-coded implemetnations of its funtionality.
> ...
>
> @@ -466,7 +458,7 @@ static int btree_insert_level(struct btree_head *head,
> struct btree_geo *geo,
> /* two identical keys are not allowed */
> BUG_ON(pos < fill && keycmp(geo, node, pos, key) == 0);
>
> - if (fill == geo->no_pairs) {
> + if (fill < 0) {
> /* need to split node */
> unsigned long *new;
The code here is a bit awkward.
: retry:
: node = find_level(head, geo, key, level);
: pos = getpos(geo, node, key);
: fill = getfill(geo, node, pos);
: /* two identical keys are not allowed */
: BUG_ON(pos < fill && keycmp(geo, node, pos, key) == 0);
:
: if (fill < 0) {
If getpos() returns -ENOENT then we're passing -ENOENT into getfill().
That might happen to work OK (or it might go BUG) but it's ugly and
unobvious.
There's a similar issue in rebalance() and in btree_remove_level():
failed to update existing getpos() callers for the new getpos() return
value semantics.
