lib/maple_tree.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
From: Steven Rostedt <rostedt@goodmis.org>
The kernel doc of mtree_insert_range() does not state if the address
represented by the "last" parameter is inclusive or exclusive. This can
lead to bugs by code that assumes it is exclusive. Explicitly state that
the parameter is inclusive, and add '[' and ']' around the word "end" to
also stress this point.
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
lib/maple_tree.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 60ae5e6fc1ee..dc9591218446 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -5730,10 +5730,12 @@ EXPORT_SYMBOL(mtree_store);
* mtree_insert_range() - Insert an entry at a given range if there is no value.
* @mt: The maple tree
* @first: The start of the range
- * @last: The end of the range
+ * @last: The [end] of the range
* @entry: The entry to store
* @gfp: The GFP_FLAGS to use for allocations.
*
+ * Note that @last is inclusive. That is, @last = @first + length - 1;
+ *
* Return: 0 on success, -EEXISTS if the range is occupied, -EINVAL on invalid
* request, -ENOMEM if memory could not be allocated.
*/
--
2.53.0
On Wed, May 06, 2026 at 10:52:18AM -0400, Steven Rostedt wrote: > From: Steven Rostedt <rostedt@goodmis.org> > > The kernel doc of mtree_insert_range() does not state if the address > represented by the "last" parameter is inclusive or exclusive. This can > lead to bugs by code that assumes it is exclusive. Explicitly state that > the parameter is inclusive, and add '[' and ']' around the word "end" to > also stress this point. > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> Reviewed-by: Alice Ryhl <aliceryhl@google.com> > lib/maple_tree.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/lib/maple_tree.c b/lib/maple_tree.c > index 60ae5e6fc1ee..dc9591218446 100644 > --- a/lib/maple_tree.c > +++ b/lib/maple_tree.c > @@ -5730,10 +5730,12 @@ EXPORT_SYMBOL(mtree_store); > * mtree_insert_range() - Insert an entry at a given range if there is no value. > * @mt: The maple tree > * @first: The start of the range > - * @last: The end of the range > + * @last: The [end] of the range > * @entry: The entry to store > * @gfp: The GFP_FLAGS to use for allocations. > * > + * Note that @last is inclusive. That is, @last = @first + length - 1; How about writing it like this? * @first: The start of the range * @last: The end of the range (inclusive) Alice
On 26/05/07 06:44AM, Alice Ryhl wrote: > On Wed, May 06, 2026 at 10:52:18AM -0400, Steven Rostedt wrote: > > From: Steven Rostedt <rostedt@goodmis.org> > > > > The kernel doc of mtree_insert_range() does not state if the address > > represented by the "last" parameter is inclusive or exclusive. This can > > lead to bugs by code that assumes it is exclusive. Explicitly state that > > the parameter is inclusive, and add '[' and ']' around the word "end" to > > also stress this point. > > > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > > Reviewed-by: Alice Ryhl <aliceryhl@google.com> > > > lib/maple_tree.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/lib/maple_tree.c b/lib/maple_tree.c > > index 60ae5e6fc1ee..dc9591218446 100644 > > --- a/lib/maple_tree.c > > +++ b/lib/maple_tree.c > > @@ -5730,10 +5730,12 @@ EXPORT_SYMBOL(mtree_store); > > * mtree_insert_range() - Insert an entry at a given range if there is no value. > > * @mt: The maple tree > > * @first: The start of the range > > - * @last: The end of the range > > + * @last: The [end] of the range > > * @entry: The entry to store > > * @gfp: The GFP_FLAGS to use for allocations. > > * > > + * Note that @last is inclusive. That is, @last = @first + length - 1; > > How about writing it like this? > > * @first: The start of the range > * @last: The end of the range (inclusive) I like this. I think the range should be identified in the initial statement. Something like this: mtree_insert_range() - Insert an entry from [first, last] if there isn't an entry within that range. > > Alice > > -- > maple-tree mailing list > maple-tree@lists.infradead.org > https://lists.infradead.org/mailman/listinfo/maple-tree
On Fri, 8 May 2026 22:51:51 +0200 "Liam R. Howlett" <liam@infradead.org> wrote: > On 26/05/07 06:44AM, Alice Ryhl wrote: > > On Wed, May 06, 2026 at 10:52:18AM -0400, Steven Rostedt wrote: > > > From: Steven Rostedt <rostedt@goodmis.org> > > > > > > The kernel doc of mtree_insert_range() does not state if the address > > > represented by the "last" parameter is inclusive or exclusive. This can > > > lead to bugs by code that assumes it is exclusive. Explicitly state that > > > the parameter is inclusive, and add '[' and ']' around the word "end" to > > > also stress this point. > > > > > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > > > > Reviewed-by: Alice Ryhl <aliceryhl@google.com> > > > > > lib/maple_tree.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/lib/maple_tree.c b/lib/maple_tree.c > > > index 60ae5e6fc1ee..dc9591218446 100644 > > > --- a/lib/maple_tree.c > > > +++ b/lib/maple_tree.c > > > @@ -5730,10 +5730,12 @@ EXPORT_SYMBOL(mtree_store); > > > * mtree_insert_range() - Insert an entry at a given range if there is no value. > > > * @mt: The maple tree > > > * @first: The start of the range > > > - * @last: The end of the range > > > + * @last: The [end] of the range > > > * @entry: The entry to store > > > * @gfp: The GFP_FLAGS to use for allocations. > > > * > > > + * Note that @last is inclusive. That is, @last = @first + length - 1; > > > > How about writing it like this? > > > > * @first: The start of the range > > * @last: The end of the range (inclusive) > > > I like this. +1. I'm also wondering if it make sense to add '(inclusive)' for 'first', too. > > I think the range should be identified in the initial statement. > Something like this: > > mtree_insert_range() - Insert an entry from [first, last] if there isn't > an entry within that range. +1. Thanks, SJ [...]
On Fri, May 08, 2026 at 05:59:46PM -0700, SeongJae Park wrote: > > > * @first: The start of the range > > > * @last: The end of the range (inclusive) > > > > > > I like this. > > +1. I'm also wondering if it make sense to add '(inclusive)' for 'first', too. I can't think of a situation in computing where we use an exclusive-first. Pure mathematics, yes, we might want to express a range as (1,2) to exclude both 1 and 2 but include 1+epsilon for all epsilon > 0. Maybe I don't work with floating point numbers enough, but I've never seen a kernel programmer make an off-by-one with the start of a range. End-of-the-range is all too common.
On Sat, 9 May 2026 09:31:13 +0100 Matthew Wilcox <willy@infradead.org> wrote: > On Fri, May 08, 2026 at 05:59:46PM -0700, SeongJae Park wrote: > > > > * @first: The start of the range > > > > * @last: The end of the range (inclusive) > > > > > > > > > I like this. > > > > +1. I'm also wondering if it make sense to add '(inclusive)' for 'first', too. > > I can't think of a situation in computing where we use an > exclusive-first. Pure mathematics, yes, we might want to express a > range as (1,2) to exclude both 1 and 2 but include 1+epsilon for all > epsilon > 0. Maybe I don't work with floating point numbers enough, > but I've never seen a kernel programmer make an off-by-one with the > start of a range. End-of-the-range is all too common. Makes sense, thank you for sharing your thought for my silly question! Thanks, SJ
© 2016 - 2026 Red Hat, Inc.