lkml.org 
[lkml]   [2023]   [Dec]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] maple_tree: do not preallocate nodes for slot stores
On Tue, Dec 12, 2023 at 11:46:40AM -0800, Sidhartha Kumar wrote:
> + /* Slot store, does not require additional nodes */
> + if ((node_size == mas->end) && ((!mt_in_rcu(mas->tree))
> + || (wr_mas.offset_end - mas->offset == 1)))
> + return 0;

Should we refactor this into a mas_is_slot_store() predicate?

A few coding-style problems with it as it's currently written:

1. The indentation on the second line is wrong. It makes the
continuation of the condition look like part of the statement. Use
extra whitespace to indent. eg:

if ((node_size == mas->end) && ((!mt_in_rcu(mas->tree))
|| (wr_mas.offset_end - mas->offset == 1)))
return 0;

2. The operator goes last on the line, not at the beginning of the
continuation line. ie:

if ((node_size == mas->end) && ((!mt_in_rcu(mas->tree)) ||
(wr_mas.offset_end - mas->offset == 1)))
return 0;

3. You don't need parens around the !mt_in_rcu(mas->tree). There's
no ambiguity to solve here:

if ((node_size == mas->end) && (!mt_in_rcu(mas->tree) ||
(wr_mas.offset_end - mas->offset == 1)))
return 0;

But I'd write it as:

if ((node_size == mas->end) &&
(!mt_in_rcu(mas->tree) || (wr_mas.offset_end - mas->offset == 1)))
return 0;

because then the whitespace matches how you're supposed to parse the
condition, and so the next person to read this code will have an easier
time of it.

\
 
 \ /
  Last update: 2023-12-12 21:58    [W:0.081 / U:0.072 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site