[PATCH] trivial - constify sched.h

[PATCH] trivial - constify sched.h

[Joe Perches]

Add const to some struct task_struct * uses

Signed-off-by: Joe Perches <joe@perches.com>

---

 include/linux/sched.h |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index ba78807..71d40a1 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1222,22 +1222,22 @@ static inline int rt_prio(int prio)
 	return 0;
 }
 
-static inline int rt_task(struct task_struct *p)
+static inline int rt_task(const struct task_struct *p)
 {
 	return rt_prio(p->prio);
 }
 
-static inline pid_t process_group(struct task_struct *tsk)
+static inline pid_t process_group(const struct task_struct *tsk)
 {
 	return tsk->signal->pgrp;
 }
 
-static inline pid_t signal_session(struct signal_struct *sig)
+static inline pid_t signal_session(const struct signal_struct *sig)
 {
 	return sig->__session;
 }
 
-static inline pid_t process_session(struct task_struct *tsk)
+static inline pid_t process_session(const struct task_struct *tsk)
 {
 	return signal_session(tsk->signal);
 }
@@ -1247,22 +1247,22 @@ static inline void set_signal_session(struct signal_struct *sig, pid_t session)
 	sig->__session = session;
 }
 
-static inline struct pid *task_pid(struct task_struct *task)
+static inline struct pid *task_pid(const struct task_struct *task)
 {
 	return task->pids[PIDTYPE_PID].pid;
 }
 
-static inline struct pid *task_tgid(struct task_struct *task)
+static inline struct pid *task_tgid(const struct task_struct *task)
 {
 	return task->group_leader->pids[PIDTYPE_PID].pid;
 }
 
-static inline struct pid *task_pgrp(struct task_struct *task)
+static inline struct pid *task_pgrp(const struct task_struct *task)
 {
 	return task->group_leader->pids[PIDTYPE_PGID].pid;
 }
 
-static inline struct pid *task_session(struct task_struct *task)
+static inline struct pid *task_session(const struct task_struct *task)
 {
 	return task->group_leader->pids[PIDTYPE_SID].pid;
 }
@@ -1275,7 +1275,7 @@ static inline struct pid *task_session(struct task_struct *task)
  * If pid_alive fails, then pointers within the task structure
  * can be stale and must not be dereferenced.
  */
-static inline int pid_alive(struct task_struct *p)
+static inline int pid_alive(const struct task_struct *p)
 {
 	return p->pids[PIDTYPE_PID].pid != NULL;
 }
@@ -1286,7 +1286,7 @@ static inline int pid_alive(struct task_struct *p)
  *
  * Check if a task structure is the first user space task the kernel created.
  */
-static inline int is_init(struct task_struct *tsk)
+static inline int is_init(const struct task_struct *tsk)
 {
 	return tsk->pid == 1;
 }
@@ -1639,7 +1639,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(struct task_struct *p)
+static inline int has_group_leader_pid(const struct task_struct *p)
 {
 	return p->pid == p->tgid;
 }
@@ -1650,7 +1650,7 @@ static inline struct task_struct *next_thread(const struct task_struct *p)
 			  struct task_struct, thread_group);
 }
 
-static inline int thread_group_empty(struct task_struct *p)
+static inline int thread_group_empty(const struct task_struct *p)
 {
 	return list_empty(&p->thread_group);
 }

Re: [PATCH] trivial - constify sched.h

[Jiri Slaby]

Joe Perches napsal(a):
> Add const to some struct task_struct * uses

Does this have any impact on generated code? What (some objdumps or something)?
Or more descriptive log, why is this about to be done, please.

> 
> Signed-off-by: Joe Perches <joe@perches.com>
> 
> ---
> 
>  include/linux/sched.h |   24 ++++++++++++------------
>  1 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index ba78807..71d40a1 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1222,22 +1222,22 @@ static inline int rt_prio(int prio)
>  	return 0;
>  }
>  
> -static inline int rt_task(struct task_struct *p)
> +static inline int rt_task(const struct task_struct *p)
>  {
>  	return rt_prio(p->prio);
>  }
>  
> -static inline pid_t process_group(struct task_struct *tsk)
> +static inline pid_t process_group(const struct task_struct *tsk)
>  {
>  	return tsk->signal->pgrp;
>  }
>  
> -static inline pid_t signal_session(struct signal_struct *sig)
> +static inline pid_t signal_session(const struct signal_struct *sig)
>  {
>  	return sig->__session;
>  }
>  
> -static inline pid_t process_session(struct task_struct *tsk)
> +static inline pid_t process_session(const struct task_struct *tsk)
>  {
>  	return signal_session(tsk->signal);
>  }
> @@ -1247,22 +1247,22 @@ static inline void set_signal_session(struct signal_struct *sig, pid_t session)
>  	sig->__session = session;
>  }
>  
> -static inline struct pid *task_pid(struct task_struct *task)
> +static inline struct pid *task_pid(const struct task_struct *task)
>  {
>  	return task->pids[PIDTYPE_PID].pid;
>  }
>  
> -static inline struct pid *task_tgid(struct task_struct *task)
> +static inline struct pid *task_tgid(const struct task_struct *task)
>  {
>  	return task->group_leader->pids[PIDTYPE_PID].pid;
>  }
>  
> -static inline struct pid *task_pgrp(struct task_struct *task)
> +static inline struct pid *task_pgrp(const struct task_struct *task)
>  {
>  	return task->group_leader->pids[PIDTYPE_PGID].pid;
>  }
>  
> -static inline struct pid *task_session(struct task_struct *task)
> +static inline struct pid *task_session(const struct task_struct *task)
>  {
>  	return task->group_leader->pids[PIDTYPE_SID].pid;
>  }
> @@ -1275,7 +1275,7 @@ static inline struct pid *task_session(struct task_struct *task)
>   * If pid_alive fails, then pointers within the task structure
>   * can be stale and must not be dereferenced.
>   */
> -static inline int pid_alive(struct task_struct *p)
> +static inline int pid_alive(const struct task_struct *p)
>  {
>  	return p->pids[PIDTYPE_PID].pid != NULL;
>  }
> @@ -1286,7 +1286,7 @@ static inline int pid_alive(struct task_struct *p)
>   *
>   * Check if a task structure is the first user space task the kernel created.
>   */
> -static inline int is_init(struct task_struct *tsk)
> +static inline int is_init(const struct task_struct *tsk)
>  {
>  	return tsk->pid == 1;
>  }
> @@ -1639,7 +1639,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(struct task_struct *p)
> +static inline int has_group_leader_pid(const struct task_struct *p)
>  {
>  	return p->pid == p->tgid;
>  }
> @@ -1650,7 +1650,7 @@ static inline struct task_struct *next_thread(const struct task_struct *p)
>  			  struct task_struct, thread_group);
>  }
>  
> -static inline int thread_group_empty(struct task_struct *p)
> +static inline int thread_group_empty(const struct task_struct *p)
>  {
>  	return list_empty(&p->thread_group);
>  }
> 
> 



-- 
Jiri Slaby (jirislaby@gmail.com)
Faculty of Informatics, Masaryk University

Re: [PATCH] trivial - constify sched.h

[Alexey Dobriyan]

On Mon, Aug 27, 2007 at 01:40:31PM -0700, Joe Perches wrote:
> Add const to some struct task_struct * uses

Why, oh, why? This way code there are more characters on screen and zero
change in vmlinux, at least here.

> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1222,22 +1222,22 @@ static inline int rt_prio(int prio)
>  	return 0;
>  }
>  
> -static inline int rt_task(struct task_struct *p)
> +static inline int rt_task(const struct task_struct *p)
>  {
>  	return rt_prio(p->prio);
>  }
	...

Re: [PATCH] trivial - constify sched.h

[Jan Engelhardt]

On Aug 28 2007 01:33, Alexey Dobriyan wrote:
>
>On Mon, Aug 27, 2007 at 01:40:31PM -0700, Joe Perches wrote:
>> Add const to some struct task_struct * uses
>
>Why, oh, why?


So that you can actually pass in a const struct task_struct * without having
to cast it back to [non-const].

Why one would have a const struct task_struct * in the first place
is a different matter.

But see http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=77adbfbf4cf96fedf9b75bb330704828c187b190


	Jan
-- 

Re: [PATCH] trivial - constify sched.h

[Satyam Sharma]

On Thu, 30 Aug 2007, Jan Engelhardt wrote:
> 
> On Aug 28 2007 01:33, Alexey Dobriyan wrote:
> >
> >On Mon, Aug 27, 2007 at 01:40:31PM -0700, Joe Perches wrote:
> >> Add const to some struct task_struct * uses
> >
> >Why, oh, why?
> 
> 
> So that you can actually pass in a const struct task_struct * without having
> to cast it back to [non-const].

... which makes zero sense, because ...

> Why one would have a const struct task_struct * in the first place
> is a different matter.

... exactly.

> But see http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=77adbfbf4cf96fedf9b75bb330704828c187b190

That commit const-ified struct timespec * or struct timeval * arguments,
which made sense because: (1) those functions really did not modify the
passed structs, and, (2) callers that pass in const struct timeval *
or const struct timespec * are indeed plausible (because one can plausibly
have const timeval/timespec structs). As the changelog suggested, those
callers were having to cast away the const qualifier before passing to
these functions to avoid seeing "passing argument discards qualifiers"
warnings. While (1) holds true for the sched.h case here, (2) does not
(and there are no warnings to shut up either).

If one really wants to go about "constifying" the kernel, then I think
there's a lot of _data_ out there that should first be made const-
qualified. xxx_ops function tables, and the like.

Re: [PATCH] trivial - constify sched.h

[Jan Engelhardt]

On Aug 31 2007 02:11, Satyam Sharma wrote:
>> So that you can actually pass in a const struct task_struct * without having
>> to cast it back to [non-const].
>
>... which makes zero sense, because ...
>
>> Why one would have a const struct task_struct * in the first place
>> is a different matter.
>
>... exactly.
>
>> But see http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=77adbfbf4cf96fedf9b75bb330704828c187b190
>
>That commit const-ified struct timespec * or struct timeval * arguments,
>which made sense because: (1) those functions really did not modify the
>passed structs, and, (2) callers that pass in const struct timeval *
>or const struct timespec * are indeed plausible (because one can plausibly
>have const timeval/timespec structs). As the changelog suggested, those
>callers
>
>were having to cast away the const qualifier before passing to
>these functions to avoid seeing "passing argument discards qualifiers"
>warnings. While (1) holds true for the sched.h case here, (2) does not
>(and there are no warnings to shut up either).

"those callers". There was _exactly one_ caller, and that was an out-of-tree
module. There were not any in-kernel callers before, and it did not generate
any warning. That is perhaps why no one had constified it before me. This does
not mean we should wait for a caller to pop up before constifying IMHO.

>If one really wants to go about "constifying" the kernel, then I think
>there's a lot of _data_ out there that should first be made const-
>qualified. xxx_ops function tables, and the like.

I think it is equally 'necessary'.



	Jan
-- 

Re: [PATCH] trivial - constify sched.h

[Satyam Sharma]

On Thu, 30 Aug 2007, Jan Engelhardt wrote:
> 
> On Aug 31 2007 02:11, Satyam Sharma wrote:
> >> So that you can actually pass in a const struct task_struct * without having
> >> to cast it back to [non-const].
> >
> >... which makes zero sense, because ...
> >
> >> Why one would have a const struct task_struct * in the first place
> >> is a different matter.
> >
> >... exactly.
> >
> >> But see http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=77adbfbf4cf96fedf9b75bb330704828c187b190
> >
> >That commit const-ified struct timespec * or struct timeval * arguments,
> >which made sense because: (1) those functions really did not modify the
> >passed structs, and, (2) callers that pass in const struct timeval *
> >or const struct timespec * are indeed plausible (because one can plausibly
> >have const timeval/timespec structs). As the changelog suggested, those
> >callers
> >
> >were having to cast away the const qualifier before passing to
> >these functions to avoid seeing "passing argument discards qualifiers"
> >warnings. While (1) holds true for the sched.h case here, (2) does not
> >(and there are no warnings to shut up either).
> 
> "those callers". There was _exactly one_ caller, and that was an out-of-tree
> module. There were not any in-kernel callers before, and it did not generate
> any warning. That is perhaps why no one had constified it before me.

You've completely missed the point -- it is _plausible_ that callers
(even if just _one_) have const timespec/timeval structs, which is why
that commit made sense as I mentioned above (to shut up the warning that
would otherwise occur). This does not hold true for the sched.h / struct
task_struct case here -- I cannot imagine a const task_struct.

> This does
> not mean we should wait for a caller to pop up before constifying IMHO.

Going about const-ifying such function arguments as in here (for the sake
of type safety, where the function does not modify that argument), could
easily lead to *zillions* of patches such as this which would have
absolutely _zero_ impact on the actual kernel that gets built.

As I said, if someone really wants to do this, please go about constifying
_data_ instead -- that would make a (positive) difference.

Re: [PATCH] trivial - constify sched.h

[Christoph Hellwig]

On Thu, Aug 30, 2007 at 10:55:49PM +0200, Jan Engelhardt wrote:
> "those callers". There was _exactly one_ caller, and that was an out-of-tree
> module. There were not any in-kernel callers before, and it did not generate
> any warning. That is perhaps why no one had constified it before me. This does
> not mean we should wait for a caller to pop up before constifying IMHO.

In this case we should just kill it instead of messing with constness.

Re: [PATCH] trivial - constify sched.h

[Matthew Wilcox]

On Fri, Aug 31, 2007 at 02:53:23PM +0100, Christoph Hellwig wrote:
> On Thu, Aug 30, 2007 at 10:55:49PM +0200, Jan Engelhardt wrote:
> > "those callers". There was _exactly one_ caller, and that was an out-of-tree
> > module. There were not any in-kernel callers before, and it did not generate
> > any warning. That is perhaps why no one had constified it before me. This does
> > not mean we should wait for a caller to pop up before constifying IMHO.
> 
> In this case we should just kill it instead of messing with constness.

I think Jan mis-spoke -- there were no in-kernel callers calling it with
a const argument.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."