Skip to contentSkip to conversation

cleanup: correct tabbing of check_kill_permission()

PATCH In review Andrew G. Morgan 3 participants 4 messages Mar 1, 2008 – Mar 2, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 cleanup: correct tabbing of check_kill_permission()

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andrew

Here is a small patch to reformat the check_kill_permission() function. It doesn't change any functionality.

There was something confusing about its tabbing previously, and it also ran over the 80 column limit.

Cheers

Andrew
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFHydmF+bHCR3gb8jsRAmehAKDVnrOMVkHPhZslnhE1tVVXl2x07gCgsAse LdjzfIwC+dmOqfDGLwnhaOo=
=pQEM
-----END PGP SIGNATURE-----

From 7dda0a7adb7ab870e01f93b6b066598665e15f7d Mon Sep 17 00:00:00 2001
From: Andrew G. Morgan <[email protected]>
Date: Mon, 25 Feb 2008 21:49:22 -0800
Subject: [PATCH] Reformat check_kill_permission() function to make it readable.

---
 kernel/signal.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)
kernel/signal.c +7 -5
index 84917fe..77f0439 100644
@@ -530,17 +530,19 @@ static int check_kill_permission(int sig, struct siginfo *info,
 	if (!valid_signal(sig))
 		return error;
 
-	if (info == SEND_SIG_NOINFO || (!is_si_special(info) && SI_FROMUSER(info))) {
-		error = audit_signal_info(sig, t); /* Let audit system see the signal */
+	if (info == SEND_SIG_NOINFO
+	    || (!is_si_special(info) && SI_FROMUSER(info))) {
+		 /* Let audit system see the signal */
+		error = audit_signal_info(sig, t);
 		if (error)
 			return error;
 		error = -EPERM;
-		if (((sig != SIGCONT) ||
-			(task_session_nr(current) != task_session_nr(t)))
+		if (((sig != SIGCONT)
+		     || (task_session_nr(current) != task_session_nr(t)))
 		    && (current->euid ^ t->suid) && (current->euid ^ t->uid)
 		    && (current->uid ^ t->suid) && (current->uid ^ t->uid)
 		    && !capable(CAP_KILL))
-		return error;
+			return error;
 	}
 
 	return security_task_kill(t, info, sig, 0);
Signature · 1 line
1.5.3.7
#1 lore raw
Quoted 6 lines · Andrew G. Morgan
kernel/signal.c +4 -0
+	if (info == SEND_SIG_NOINFO
+	    || (!is_si_special(info) && SI_FROMUSER(info))) {
+		 /* Let audit system see the signal */
+		error = audit_signal_info(sig, t);
 		if (error)
 			return error;

kernel/signal.c static int check_kill_permission(int sig, struct siginfo *info, · line 545

Quoted 10 lines
kernel/signal.c +3 -3
 		error = -EPERM;
-		if (((sig != SIGCONT) ||
-			(task_session_nr(current) != task_session_nr(t)))
+		if (((sig != SIGCONT)
+		     || (task_session_nr(current) != task_session_nr(t)))
 		    && (current->euid ^ t->suid) && (current->euid ^ t->uid)
 		    && (current->uid ^ t->suid) && (current->uid ^ t->uid)
 		    && !capable(CAP_KILL))
-		return error;
+			return error;

If you start reformmating it please get it completely right:

	if (info == SEND_SIG_NOINFO ||
	    (!is_si_special(info) && SI_FROMUSER(info))) {
		/* Let audit system see the signal */
		error = audit_signal_info(sig, t);
 		if (error)
 			return error;
 		error = -EPERM;
		if ((sig != SIGCONT ||
		     (task_session_nr(current) != task_session_nr(t))) &&
		    (current->euid ^ t->suid) && (current->euid ^ t->uid) &&
		    (current->uid ^ t->suid) && (current->uid ^ t->uid) &&
		    !capable(CAP_KILL))
			return error;

}

#2 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Christoph,

I ran it through scripts/checkpatch.pl - it said it was ok - and I see that other places in this file contain code of this style - do_sigaltstack() for example.

Could you give me a reference for "completely right"? Its not obvious to me that Documentation/CodingStyle is what you are using as a reference.

Thanks

Andrew

Christoph Hellwig wrote:

| On Sat, Mar 01, 2008 at 02:32:38PM -0800, Andrew G. Morgan wrote:
|> +	if (info == SEND_SIG_NOINFO
|> +	    || (!is_si_special(info) && SI_FROMUSER(info))) {
|> +		 /* Let audit system see the signal */
|> +		error = audit_signal_info(sig, t);
|>  		if (error)
|>  			return error;
|>  		error = -EPERM;
|> -		if (((sig != SIGCONT) ||
|> -			(task_session_nr(current) != task_session_nr(t)))
|> +		if (((sig != SIGCONT)
|> +		     || (task_session_nr(current) != task_session_nr(t)))
|>  		    && (current->euid ^ t->suid) && (current->euid ^ t->uid)
|>  		    && (current->uid ^ t->suid) && (current->uid ^ t->uid)
|>  		    && !capable(CAP_KILL))
|> -		return error;
|> +			return error;
|
| If you start reformmating it please get it completely right:
|
| 	if (info == SEND_SIG_NOINFO ||
| 	    (!is_si_special(info) && SI_FROMUSER(info))) {
| 		/* Let audit system see the signal */
| 		error = audit_signal_info(sig, t);
|  		if (error)
|  			return error;
|  		error = -EPERM;
| 		if ((sig != SIGCONT ||
| 		     (task_session_nr(current) != task_session_nr(t))) &&
| 		    (current->euid ^ t->suid) && (current->euid ^ t->uid) &&
| 		    (current->uid ^ t->suid) && (current->uid ^ t->uid) &&
| 		    !capable(CAP_KILL))
| 			return error;
|  	}

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFHyjfn+bHCR3gb8jsRAuilAJ9QJKsEQz66UrFtLOCcHVokGcVV7wCeP0HE ftOI2APLX1YpWiznyhLfN6U=
=HGa0
-----END PGP SIGNATURE-----

#3 lore raw
Quoted 3 lines · Andrew G. Morgan

...
I ran it through scripts/checkpatch.pl - it said it was ok
...

checkpatch is just a tool pointing to some common mistakes, not the definite authority for proving that code is stylistically correct.

Similar to the fact that I hope you wouldn't claim your code was working correctly only because gcc compiled it without warnings...

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed