Linus, please revert commit a8972ccf00b7184a743eb6cd9bc7f3443357910c aka
"sched: constify sched.h" or apply the following patch instead.
[PATCH] De-constify sched.h
1) Patch doesn't change any code here, so gcc is already smart enough to "feel"
constness in such simple functions.
2) There is no such thing as const task_struct. Anyone who think otherwise
deserves compiler warning.
Signed-off-by: Alexey Dobriyan <[email protected]>
---
include/linux/sched.h | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1201,7 +1201,7 @@ static inline int rt_prio(int prio)
return 0;
}
-static inline int rt_task(const struct task_struct *p)
+static inline int rt_task(struct task_struct *p)
{
return rt_prio(p->prio);
}
@@ -1216,22 +1216,22 @@ static inline void set_task_pgrp(struct task_struct *tsk, pid_t pgrp)
tsk->signal->__pgrp = pgrp;
}
-static inline struct pid *task_pid(const struct task_struct *task)
+static inline struct pid *task_pid(struct task_struct *task)
{
return task->pids[PIDTYPE_PID].pid;
}
-static inline struct pid *task_tgid(const struct task_struct *task)
+static inline struct pid *task_tgid(struct task_struct *task)
{
return task->group_leader->pids[PIDTYPE_PID].pid;
}
-static inline struct pid *task_pgrp(const struct task_struct *task)
+static inline struct pid *task_pgrp(struct task_struct *task)
{
return task->group_leader->pids[PIDTYPE_PGID].pid;
}
-static inline struct pid *task_session(const struct task_struct *task)
+static inline struct pid *task_session(struct task_struct *task)
{
return task->group_leader->pids[PIDTYPE_SID].pid;
}
@@ -1260,7 +1260,7 @@ struct pid_namespace;
* see also pid_nr() etc in include/linux/pid.h
*/
-static inline pid_t task_pid_nr(const struct task_struct *tsk)
+static inline pid_t task_pid_nr(struct task_struct *tsk)
{
return tsk->pid;
}
@@ -1273,7 +1273,7 @@ static inline pid_t task_pid_vnr(struct task_struct *tsk)
}
-static inline pid_t task_tgid_nr(const struct task_struct *tsk)
+static inline pid_t task_tgid_nr(struct task_struct *tsk)
{
return tsk->tgid;
}
@@ -1286,7 +1286,7 @@ static inline pid_t task_tgid_vnr(struct task_struct *tsk)
}
-static inline pid_t task_pgrp_nr(const struct task_struct *tsk)
+static inline pid_t task_pgrp_nr(struct task_struct *tsk)
{
return tsk->signal->__pgrp;
}
@@ -1299,7 +1299,7 @@ static inline pid_t task_pgrp_vnr(struct task_struct *tsk)
}
-static inline pid_t task_session_nr(const struct task_struct *tsk)
+static inline pid_t task_session_nr(struct task_struct *tsk)
{
return tsk->signal->__session;
}
@@ -1326,7 +1326,7 @@ static inline pid_t task_ppid_nr_ns(struct task_struct *tsk,
* If pid_alive fails, then pointers within the task structure
* can be stale and must not be dereferenced.
*/
-static inline int pid_alive(const struct task_struct *p)
+static inline int pid_alive(struct task_struct *p)
{
return p->pids[PIDTYPE_PID].pid != NULL;
}
@@ -1337,7 +1337,7 @@ static inline int pid_alive(const struct task_struct *p)
*
* Check if a task structure is the first user space task the kernel created.
*/
-static inline int is_global_init(const struct task_struct *tsk)
+static inline int is_global_init(struct task_struct *tsk)
{
return tsk->pid == 1;
}
@@ -1474,7 +1474,7 @@ extern int rt_mutex_getprio(struct task_struct *p);
extern void rt_mutex_setprio(struct task_struct *p, int prio);
extern void rt_mutex_adjust_pi(struct task_struct *p);
#else
-static inline int rt_mutex_getprio(const struct task_struct *p)
+static inline int rt_mutex_getprio(struct task_struct *p)
{
return p->normal_prio;
}
@@ -1726,7 +1726,7 @@ extern void wait_task_inactive(struct task_struct * p);
* all we care about is that we have a task with the appropriate
* pid, we don't actually care if we have the right task.
*/
-static inline int has_group_leader_pid(const struct task_struct *p)
+static inline int has_group_leader_pid(struct task_struct *p)
{
return p->pid == p->tgid;
}
@@ -1743,7 +1743,7 @@ static inline struct task_struct *next_thread(const struct task_struct *p)
struct task_struct, thread_group);
}
-static inline int thread_group_empty(const struct task_struct *p)
+static inline int thread_group_empty(struct task_struct *p)
{
return list_empty(&p->thread_group);
}
On Fri, 26 Oct 2007, Alexey Dobriyan wrote:
> 2) There is no such thing as const task_struct. Anyone who think otherwise
> deserves compiler warning.
>
A 'const struct task_struct *' can be used as an annotation to mean that
no member of the struct is modified through that pointer, so it's
perfectly acceptable to qualify formals in that manner.
From: David Rientjes <[email protected]>
Date: Fri, 26 Oct 2007 02:42:30 -0700 (PDT)
> On Fri, 26 Oct 2007, Alexey Dobriyan wrote:
>
> > 2) There is no such thing as const task_struct. Anyone who think otherwise
> > deserves compiler warning.
> >
>
> A 'const struct task_struct *' can be used as an annotation to mean that
> no member of the struct is modified through that pointer, so it's
> perfectly acceptable to qualify formals in that manner.
But in one of the cases he un-const's the code does modify
the object through the pointer. At least that one should
be reverted since the annotation is wrong.
* Alexey Dobriyan <[email protected]> wrote:
> Linus, please revert commit a8972ccf00b7184a743eb6cd9bc7f3443357910c aka
> "sched: constify sched.h" or apply the following patch instead.
>
> [PATCH] De-constify sched.h
firstly, thank you for not Cc:-ing me. I learned about this revert only
once it was done deal in Linus' tree. You objected to the patch
originally when it was submitted to lkml, and plausible arguments were
presented against your (bogus) objection:
http://article.gmane.org/gmane.linux.kernel.janitors/14623
in that thread, two months ago, you essentially conceded Jan
Engelhardt's argument by not replying to his points, so i kept Joe's
patch. And now you continue this "discussion" by asking for a revert and
not Cc:-ing me, Joe or Jan - which is quite sneaky.
> 1) Patch doesn't change any code here, so gcc is already smart enough
> to "feel" constness in such simple functions.
the const markers indeed had no real purpose in terms of code generation
(gcc can figure it out whether something is modified by an inline
function), but they had a documentation/intent purpose, and they are
plausible if a non-inlined function uses a const task struct in the
future. For example any of these functions could be un-inlined and could
use (internally) one of the remaining inlines - in that case code
generation gets better from the constifying as well. There are also a
good deal of helper functions around task struct which could be marked
with const.
we do have other inline functions in include/linux/*.h with 'const'
arguments, so marking arguments with const is not without precedent and
this was pointed out to you in the original discussion.
> 2) There is no such thing as const task_struct. Anyone who think
> otherwise deserves compiler warning.
-ENOPARSE. 'const struct task_struct *' says that the task struct is not
supposed to be modified within that (inline) function. That _does_ make
sense.
so unless i'm missing something, your request for revert was pretty
rude, technically incorrect and you also tried to circumvent the normal
course of discussion. I have no strong feelings either way technically
(the patch is borderline - we dont actively pass around const
task_struct pointers at the moment - but we could start doing so, if we
had the constification), but i do have strong feelings against the kind
of behavior you showed here.
Ingo