[PATCH] Remove errors caught by checkpatch.pl in kernel/signal.c

[PATCH] Remove errors caught by checkpatch.pl in kernel/signal.c

[Manish Katiyar]

Hi,

Below patch removes some errors generated by checkpatch.pl in
kernel/signal.c. Caught by Ingo's code-quality script.


Signed-off-by: Manish Katiyar <mkatiyar@gmail.com>
---
 kernel/signal.c |   47 +++++++++++++++++++++++------------------------
 1 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index e737597..9fde101 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -89,20 +89,20 @@ static inline int has_pending_signals(sigset_t
*signal, sigset_t *blocked)
 	switch (_NSIG_WORDS) {
 	default:
 		for (i = _NSIG_WORDS, ready = 0; --i >= 0 ;)
-			ready |= signal->sig[i] &~ blocked->sig[i];
+			ready |= signal->sig[i] & ~blocked->sig[i];
 		break;

-	case 4: ready  = signal->sig[3] &~ blocked->sig[3];
-		ready |= signal->sig[2] &~ blocked->sig[2];
-		ready |= signal->sig[1] &~ blocked->sig[1];
-		ready |= signal->sig[0] &~ blocked->sig[0];
+	case 4: ready  = signal->sig[3] & ~blocked->sig[3];
+		ready |= signal->sig[2] & ~blocked->sig[2];
+		ready |= signal->sig[1] & ~blocked->sig[1];
+		ready |= signal->sig[0] & ~blocked->sig[0];
 		break;

-	case 2: ready  = signal->sig[1] &~ blocked->sig[1];
-		ready |= signal->sig[0] &~ blocked->sig[0];
+	case 2: ready  = signal->sig[1] & ~blocked->sig[1];
+		ready |= signal->sig[0] & ~blocked->sig[0];
 		break;

-	case 1: ready  = signal->sig[0] &~ blocked->sig[0];
+	case 1: ready  = signal->sig[0] & ~blocked->sig[0];
 	}
 	return ready !=	0;
 }
@@ -156,22 +156,22 @@ int next_signal(struct sigpending *pending,
sigset_t *mask)
 	switch (_NSIG_WORDS) {
 	default:
 		for (i = 0; i < _NSIG_WORDS; ++i, ++s, ++m)
-			if ((x = *s &~ *m) != 0) {
+			if ((x = *s & ~*m) != 0) {
 				sig = ffz(~x) + i*_NSIG_BPW + 1;
 				break;
 			}
 		break;

-	case 2: if ((x = s[0] &~ m[0]) != 0)
+	case 2: if ((x = s[0] & ~m[0]) != 0)
 			sig = 1;
-		else if ((x = s[1] &~ m[1]) != 0)
+		else if ((x = s[1] & ~m[1]) != 0)
 			sig = _NSIG_BPW + 1;
 		else
 			break;
 		sig += ffz(~x);
 		break;

-	case 1: if ((x = *s &~ *m) != 0)
+	case 1: if ((x = *s & ~*m) != 0)
 			sig = ffz(~x) + 1;
 		break;
 	}
@@ -414,7 +414,7 @@ static int __dequeue_signal(struct sigpending
*pending, sigset_t *mask,
 }

 /*
- * Dequeue a signal and return the element to the caller, which is
+ * Dequeue a signal and return the element to the caller, which is
  * expected to free it.
  *
  * All callers have to hold the siglock.
@@ -914,7 +914,7 @@ static void print_fatal_signal(struct pt_regs
*regs, int signr)

 static int __init setup_print_fatal_signals(char *str)
 {
-	get_option (&str, &print_fatal_signals);
+	get_option(&str, &print_fatal_signals);

 	return 1;
 }
@@ -1164,7 +1164,7 @@ static int kill_something_info(int sig, struct
siginfo *info, pid_t pid)
 				pid ? find_vpid(-pid) : task_pgrp(current));
 	} else {
 		int retval = 0, count = 0;
-		struct task_struct * p;
+		struct task_struct *p;

 		for_each_process(p) {
 			if (task_pid_vnr(p) > 1 &&
@@ -1264,19 +1264,18 @@ EXPORT_SYMBOL(kill_pid);
  * These functions support sending signals using preallocated sigqueue
  * structures.  This is needed "because realtime applications cannot
  * afford to lose notifications of asynchronous events, like timer
- * expirations or I/O completions".  In the case of Posix Timers
+ * expirations or I/O completions".  In the case of Posix Timers
  * we allocate the sigqueue structure from the timer_create.  If this
  * allocation fails we are able to report the failure to the application
  * with an EAGAIN error.
  */
-
 struct sigqueue *sigqueue_alloc(void)
 {
 	struct sigqueue *q;

 	if ((q = __sigqueue_alloc(current, GFP_KERNEL, 0)))
 		q->flags |= SIGQUEUE_PREALLOC;
-	return(q);
+	return q;
 }

 void sigqueue_free(struct sigqueue *q)
@@ -1374,7 +1373,7 @@ int do_notify_parent(struct task_struct *tsk, int sig)
  	BUG_ON(task_is_stopped_or_traced(tsk));

 	BUG_ON(!tsk->ptrace &&
-	       (tsk->group_leader != tsk || !thread_group_empty(tsk)));
+		(tsk->group_leader != tsk || !thread_group_empty(tsk)));

 	info.si_signo = sig;
 	info.si_errno = 0;
@@ -2072,7 +2071,7 @@ long do_sigpending(void __user *set, unsigned
long sigsetsize)

 out:
 	return error;
-}	
+}

 SYSCALL_DEFINE2(rt_sigpending, sigset_t __user *, set, size_t, sigsetsize)
 {
@@ -2085,7 +2084,7 @@ int copy_siginfo_to_user(siginfo_t __user *to,
siginfo_t *from)
 {
 	int err;

-	if (!access_ok (VERIFY_WRITE, to, sizeof(siginfo_t)))
+	if (!access_ok(VERIFY_WRITE, to, sizeof(siginfo_t)))
 		return -EFAULT;
 	if (from->si_code < 0)
 		return __copy_to_user(to, from, sizeof(siginfo_t))
@@ -2161,7 +2160,7 @@ SYSCALL_DEFINE4(rt_sigtimedwait, const sigset_t
__user *, uthese,

 	if (copy_from_user(&these, uthese, sizeof(these)))
 		return -EFAULT;
-		
+
 	/*
 	 * Invert the set of allowed signals to get those we
 	 * want to block.
@@ -2364,7 +2363,7 @@ int do_sigaction(int sig, struct k_sigaction
*act, struct k_sigaction *oact)
 }

 int
-do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss,
unsigned long sp)
+do_sigaltstack(const stack_t __user *uss, stack_t __user *uoss,
unsigned long sp)
 {
 	stack_t oss;
 	int error;
@@ -2479,7 +2478,7 @@ SYSCALL_DEFINE3(sigprocmask, int, how,
old_sigset_t __user *, set,
 			goto set_old;
 	} else if (oset) {
 		old_set = current->blocked.sig[0];
-	set_old:
+set_old:
 		error = -EFAULT;
 		if (copy_to_user(oset, &old_set, sizeof(*oset)))
 			goto out;
-- 
1.5.4.3


Thanks -
Manish

Re: [PATCH] Remove errors caught by checkpatch.pl in kernel/signal.c

[Ingo Molnar]

* Manish Katiyar <mkatiyar@gmail.com> wrote:

> Hi,
> 
> Below patch removes some errors generated by checkpatch.pl in
> kernel/signal.c. Caught by Ingo's code-quality script.
> 
> 
> Signed-off-by: Manish Katiyar <mkatiyar@gmail.com>
> ---
>  kernel/signal.c |   47 +++++++++++++++++++++++------------------------
>  1 files changed, 23 insertions(+), 24 deletions(-)

Note, your patch has line-wrap problems, such as:

> +++ b/kernel/signal.c
> @@ -89,20 +89,20 @@ static inline int has_pending_signals(sigset_t
> *signal, sigset_t *blocked)
>  	switch (_NSIG_WORDS) {

Causing:

 patch: **** malformed patch at line 23: *signal, sigset_t *blocked)

See Documentation/email-clients.txt.

Also, kernel/signal.c needs a thorough cleanup, and your patch only
handles about a third of the errors+warnings:

 before: total: 61 errors, 28 warnings, 3 checks, 2615 lines checked
  after: total: 30 errors, 24 warnings, 3 checks, 2614 lines checked

If then it's best to bring the count down very close to zero (so
that only the obvious checkpatch false positives are left), and also
have a really good human-coder look at signal.c's:

 - structure and code flow
 - variable naming
 - include files section
 - general splitup and function ordering
 - comment style consistency

etc. - to turn it into a really modern, nice-to-look-at and hackable
Linux kernel file.

The checkpatch motivated cleanups really only tell us half of the
story that is usually in such a long-forgotten file - and by fixing
the checkpatch warnings only it gives us a false impression of
cleanliness. So the checkpatch fixes should go hand in hand with
more grounds-up cleanups. (which can all still stay at the pure
style level, so that the .o output does not change - for easy
verification.)

	Ingo