2008-02-03 03:04:50

by Rabin Vincent

[permalink] [raw]
Subject: [PATCH] Remove unneeded code in sys_getpriority

This check is not required because the condition is always true.

Signed-off-by: Rabin Vincent <[email protected]>
---
kernel/sys.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/kernel/sys.c b/kernel/sys.c
index d1fe71e..a001974 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -212,11 +212,8 @@ asmlinkage long sys_getpriority(int which, int who)
p = find_task_by_vpid(who);
else
p = current;
- if (p) {
- niceval = 20 - task_nice(p);
- if (niceval > retval)
- retval = niceval;
- }
+ if (p)
+ retval = 20 - task_nice(p);
break;
case PRIO_PGRP:
if (who)
--
1.5.3.8


2008-02-03 09:54:57

by Frank Seidel

[permalink] [raw]
Subject: Re: [PATCH] Remove unneeded code in sys_getpriority

On Sunday 03 February 2008 04:04, Rabin Vincent wrote:
> This check is not required because the condition is always true.
> ...
> - if (niceval > retval)
> - retval = niceval;
> + retval = 20 - task_nice(p);

Thats surely correct, but on the other hand currently those
case blocks are quite independet of their possition/could easily
be rearranged now .. or think of another case is put ahead.
Then this could mess up things.

Thanks,
Frank

2008-02-03 17:59:31

by Rabin Vincent

[permalink] [raw]
Subject: Re: [PATCH] Remove unneeded code in sys_getpriority

On Sun, Feb 03, 2008 at 10:54:45AM +0100, Frank Seidel wrote:
> On Sunday 03 February 2008 04:04, Rabin Vincent wrote:
> > This check is not required because the condition is always true.
> > ...
> > - if (niceval > retval)
> > - retval = niceval;
> > + retval = 20 - task_nice(p);
>
> Thats surely correct, but on the other hand currently those
> case blocks are quite independet of their possition/could easily
> be rearranged now .. or think of another case is put ahead.
> Then this could mess up things.

Do you mean the PRIO_* cases in the switch? They're still independent
of position after the patch because they don't fall through.

> Thanks,
> Frank

Rabin

2008-02-03 18:10:41

by Frank Seidel

[permalink] [raw]
Subject: Re: [PATCH] Remove unneeded code in sys_getpriority

On Sunday 03 February 2008 18:58, Rabin Vincent wrote:
> Do you mean the PRIO_* cases in the switch? They're still independent
> of position after the patch because they don't fall through.

Yes, sure, this is fully correct now. Just if somehting whatsoever
is put ahead touching retval one need to take care of this here.
But as i don't have enough experience to know if this likely at all i'm
fully fine with it. Just wanted to note one should think about if
its worth the change.

Thanks,
Frank