diff mbox

[01/20] rbtree: add const qualifier to some functions

Message ID 20170617093253.3990-2-kpraveen.lkml@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Praveen Kumar June 17, 2017, 9:32 a.m. UTC 
The 'rb_first()', 'rb_last()', 'rb_next()' and 'rb_prev()' calls
take a pointer to an RB node or RB root. They do not change the
pointed objects, so add a 'const' qualifier in order to make life
of the users of these functions easier.

Indeed, if I have my own constant pointer &const struct my_type *p,
and I call 'rb_next(&p->rb)', I get a GCC warning:

warning: passing argument 1 of ‘rb_next’ discards qualifiers from pointer target
type

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
[Linux commit f4b477c47332367d35686bd2b808c2156b96d7c7]

Ported to Xen.

Signed-off-by: Praveen Kumar <kpraveen.lkml@gmail.com>
---
 xen/common/rbtree.c      | 12 ++++++------
 xen/include/xen/rbtree.h |  8 ++++----
 2 files changed, 10 insertions(+), 10 deletions(-)

Comments

Jan Beulich June 19, 2017, 1:49 p.m. UTC | #1 
>>> On 17.06.17 at 11:32, <kpraveen.lkml@gmail.com> wrote:
> The 'rb_first()', 'rb_last()', 'rb_next()' and 'rb_prev()' calls
> take a pointer to an RB node or RB root. They do not change the
> pointed objects, so add a 'const' qualifier in order to make life
> of the users of these functions easier.
> 
> Indeed, if I have my own constant pointer &const struct my_type *p,
> and I call 'rb_next(&p->rb)', I get a GCC warning:
> 
> warning: passing argument 1 of ‘rb_next’ discards qualifiers from pointer target
> type
> 
> Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> [Linux commit f4b477c47332367d35686bd2b808c2156b96d7c7]
> 
> Ported to Xen.
> 
> Signed-off-by: Praveen Kumar <kpraveen.lkml@gmail.com>

This looks okay now from a content pov, but I still have a question
and a remark.

Question: Who's the original author? According to the Linux commit,
it's Artem, but without an explicit From: tag I think anyone trying to
"git am" you mail would put you in as the author. With this taken
care of (which the committer may be willing to do)
Acked-by: Jan Beulich <jbeulich@suse.com>

Remark: You've sent v3 of the series. This patch has no version
indicator at all, and most other patches say v2. This is all quite
inconsistent, and prevents easily identifying the various pieces
belonging together when not using a threaded view. If you add
new patches to a series, don't start them at v1. Instead in the
brief revision info (after the first --- marker) simply say "New in
v3."

Jan
Praveen Kumar June 19, 2017, 2:09 p.m. UTC | #2 
Hi Jan,

On Mon, Jun 19, 2017 at 7:19 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 17.06.17 at 11:32, <kpraveen.lkml@gmail.com> wrote:
>> The 'rb_first()', 'rb_last()', 'rb_next()' and 'rb_prev()' calls
>> take a pointer to an RB node or RB root. They do not change the
>> pointed objects, so add a 'const' qualifier in order to make life
>> of the users of these functions easier.
>>
>> Indeed, if I have my own constant pointer &const struct my_type *p,
>> and I call 'rb_next(&p->rb)', I get a GCC warning:
>>
>> warning: passing argument 1 of ‘rb_next’ discards qualifiers from pointer target
>> type
>>
>> Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
>> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
>> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
>> [Linux commit f4b477c47332367d35686bd2b808c2156b96d7c7]
>>
>> Ported to Xen.
>>
>> Signed-off-by: Praveen Kumar <kpraveen.lkml@gmail.com>
>
> This looks okay now from a content pov, but I still have a question
> and a remark.
>
> Question: Who's the original author? According to the Linux commit,
> it's Artem, but without an explicit From: tag I think anyone trying to
> "git am" you mail would put you in as the author. With this taken
> care of (which the committer may be willing to do)
> Acked-by: Jan Beulich <jbeulich@suse.com>
>

Thanks for your input.
Pardon me, I am new to the forum. The Ack you added is only for this patch.

Also, do you want me to add 'From' and resend ?
To be precise in below order  :

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
[Linux commit f4b477c47332367d35686bd2b808c2156b96d7c7]

Ported to Xen.

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Signed-off-by: Praveen Kumar <kpraveen.lkml@gmail.com>

Please do correct me if my understanding is wrong. Thanks.

> Remark: You've sent v3 of the series. This patch has no version
> indicator at all, and most other patches say v2. This is all quite
> inconsistent, and prevents easily identifying the various pieces
> belonging together when not using a threaded view. If you add
> new patches to a series, don't start them at v1. Instead in the
> brief revision info (after the first --- marker) simply say "New in
> v3."
>

Sure, my mistake. I was not aware of the same. I will take care in
future patches.
Thanks for your input.

Regards,

~Praveen.
Jan Beulich June 19, 2017, 2:47 p.m. UTC | #3 
>>> On 19.06.17 at 16:09, <kpraveen.lkml@gmail.com> wrote:
> On Mon, Jun 19, 2017 at 7:19 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 17.06.17 at 11:32, <kpraveen.lkml@gmail.com> wrote:
>>> The 'rb_first()', 'rb_last()', 'rb_next()' and 'rb_prev()' calls
>>> take a pointer to an RB node or RB root. They do not change the
>>> pointed objects, so add a 'const' qualifier in order to make life
>>> of the users of these functions easier.
>>>
>>> Indeed, if I have my own constant pointer &const struct my_type *p,
>>> and I call 'rb_next(&p->rb)', I get a GCC warning:
>>>
>>> warning: passing argument 1 of ‘rb_next’ discards qualifiers from pointer target
>>> type
>>>
>>> Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
>>> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
>>> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
>>> [Linux commit f4b477c47332367d35686bd2b808c2156b96d7c7]
>>>
>>> Ported to Xen.
>>>
>>> Signed-off-by: Praveen Kumar <kpraveen.lkml@gmail.com>
>>
>> This looks okay now from a content pov, but I still have a question
>> and a remark.
>>
>> Question: Who's the original author? According to the Linux commit,
>> it's Artem, but without an explicit From: tag I think anyone trying to
>> "git am" you mail would put you in as the author. With this taken
>> care of (which the committer may be willing to do)
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>>
> 
> Thanks for your input.
> Pardon me, I am new to the forum. The Ack you added is only for this patch.

Yes.

> Also, do you want me to add 'From' and resend ?

As said, I think the committer may be able to take care of this. So
re-sending just because of this is likely not required.

> To be precise in below order  :
> 
> Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> [Linux commit f4b477c47332367d35686bd2b808c2156b96d7c7]
> 
> Ported to Xen.
> 
> From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> Signed-off-by: Praveen Kumar <kpraveen.lkml@gmail.com>

Normally From: would go first (before the description text) I think.

Jan
Dario Faggioli June 19, 2017, 4:39 p.m. UTC | #4 
On Sat, 2017-06-17 at 15:02 +0530, Praveen Kumar wrote:
> The 'rb_first()', 'rb_last()', 'rb_next()' and 'rb_prev()' calls
> take a pointer to an RB node or RB root. They do not change the
> pointed objects, so add a 'const' qualifier in order to make life
> of the users of these functions easier.
> 
> Indeed, if I have my own constant pointer &const struct my_type *p,
> and I call 'rb_next(&p->rb)', I get a GCC warning:
> 
> warning: passing argument 1 of ‘rb_next’ discards qualifiers from
> pointer target
> type
> 
> Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> [Linux commit f4b477c47332367d35686bd2b808c2156b96d7c7]
> 
> Ported to Xen.
> 
> Signed-off-by: Praveen Kumar <kpraveen.lkml@gmail.com>
>
With authorship (the From: field) fixed as Jan suggested,

Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>

(The remark on the authorship applies of course to all the patches,
even if I don't explicitly state it again).

Dario
diff mbox

Patch

diff --git a/xen/common/rbtree.c b/xen/common/rbtree.c
index 3328960d56..d86b5f25c0 100644
--- a/xen/common/rbtree.c
+++ b/xen/common/rbtree.c
@@ -291,7 +291,7 @@  EXPORT_SYMBOL(rb_erase);
 /*
  * This function returns the first node (in sort order) of the tree.
  */
-struct rb_node *rb_first(struct rb_root *root)
+struct rb_node *rb_first(const struct rb_root *root)
 {
     struct rb_node *n;
 
@@ -304,7 +304,7 @@  struct rb_node *rb_first(struct rb_root *root)
 }
 EXPORT_SYMBOL(rb_first);
 
-struct rb_node *rb_last(struct rb_root *root)
+struct rb_node *rb_last(const struct rb_root *root)
 {
     struct rb_node *n;
 
@@ -317,7 +317,7 @@  struct rb_node *rb_last(struct rb_root *root)
 }
 EXPORT_SYMBOL(rb_last);
 
-struct rb_node *rb_next(struct rb_node *node)
+struct rb_node *rb_next(const struct rb_node *node)
 {
     struct rb_node *parent;
 
@@ -330,7 +330,7 @@  struct rb_node *rb_next(struct rb_node *node)
         node = node->rb_right; 
         while (node->rb_left)
             node=node->rb_left;
-        return node;
+        return (struct rb_node *)node;
     }
 
     /* No right-hand children.  Everything down and left is
@@ -346,7 +346,7 @@  struct rb_node *rb_next(struct rb_node *node)
 }
 EXPORT_SYMBOL(rb_next);
 
-struct rb_node *rb_prev(struct rb_node *node)
+struct rb_node *rb_prev(const struct rb_node *node)
 {
     struct rb_node *parent;
 
@@ -359,7 +359,7 @@  struct rb_node *rb_prev(struct rb_node *node)
         node = node->rb_left; 
         while (node->rb_right)
             node=node->rb_right;
-        return node;
+        return (struct rb_node *)node;
     }
 
     /* No left-hand children. Go up till we find an ancestor which
diff --git a/xen/include/xen/rbtree.h b/xen/include/xen/rbtree.h
index f93c4d5823..3eb527eb37 100644
--- a/xen/include/xen/rbtree.h
+++ b/xen/include/xen/rbtree.h
@@ -60,10 +60,10 @@  extern void rb_insert_color(struct rb_node *, struct rb_root *);
 extern void rb_erase(struct rb_node *, struct rb_root *);
 
 /* Find logical next and previous nodes in a tree */
-extern struct rb_node *rb_next(struct rb_node *);
-extern struct rb_node *rb_prev(struct rb_node *);
-extern struct rb_node *rb_first(struct rb_root *);
-extern struct rb_node *rb_last(struct rb_root *);
+extern struct rb_node *rb_next(const struct rb_node *);
+extern struct rb_node *rb_prev(const struct rb_node *);
+extern struct rb_node *rb_first(const struct rb_root *);
+extern struct rb_node *rb_last(const struct rb_root *);
 
 /* Fast replacement of a single node without remove/rebalance/add/rebalance */
 extern void rb_replace_node(struct rb_node *victim, struct rb_node *new,