linux-kernel - Re: [PATCH 01/30] headers: fixup cred.h, ipc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090410084747.GB17962@elte.hu>
Date:	Fri, 10 Apr 2009 10:47:47 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Alexey Dobriyan <adobriyan@...il.com>
Cc:	akpm@...ux-foundation.org, containers@...ts.linux-foundation.org,
	xemul@...allels.com, serue@...ibm.com, dave@...ux.vnet.ibm.com,
	orenl@...columbia.edu, hch@...radead.org,
	torvalds@...ux-foundation.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 01/30] headers: fixup cred.h, ipc_namespace.h

* Alexey Dobriyan <adobriyan@...il.com> wrote:

> cred.h uses __init
> ipc_namespace.h uses kern_ipc_perm.
> 
> Signed-off-by: Alexey Dobriyan <adobriyan@...il.com>
> ---
> 
>  include/linux/cred.h          |    1 +
>  include/linux/ipc_namespace.h |    2 ++
>  2 files changed, 3 insertions(+)
> 
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -13,6 +13,7 @@
>  #define _LINUX_CRED_H
>  
>  #include <linux/capability.h>
> +#include <linux/init.h>
>  #include <linux/key.h>
>  #include <asm/atomic.h>
>  
> --- a/include/linux/ipc_namespace.h
> +++ b/include/linux/ipc_namespace.h
> @@ -6,6 +6,8 @@
>  #include <linux/rwsem.h>
>  #include <linux/notifier.h>
>  
> +struct kern_ipc_perm;
> +

Please, not yet another forward declaration...

I believe this should be cleaned up properly, instead of sprinkling 
the kernel with random placed forward declarations.

In a cleanly structured piace of code there's rarely any need for 
forward declarations like this. There's two basic header types 
needed:

   foo_types.h       declares all the data types and constants
                     [no complex inline functions]

   foo.h             externs, complex inlines

foo.h includes all foo_types.h and bar_types.h headers it needs.

The foo_types.h file includes all other bar_types.h headers it 
depends on for its data type definitions.

foo_types.h never includes a bar.h file.

The end result will be a clean hieararchy of data types, separated 
from methods. Complex inlines dont create the incestous 
cross-dependency hell anymore and dont contaminate the foo_types.h 
files.

Yes, this is quite a bit of work, but this has been done before for 
several complex headers so it's possible to do it. Also, you 
namespace guys compounded this particular IPC headers sub-problem to 
begin with:

 | commit ae5e1b22f17983da929a0d0178896269e19da186
 | Author: Pavel Emelyanov <xemul@...nvz.org>
 | Date:   Fri Feb 8 04:18:22 2008 -0800
 |
 |     namespaces: move the IPC namespace under IPC_NS option

So when touching the IPC header code please clean it up all 
properly. We need separate ipc_types.h and ipc.h headers with clean 
rules and a clean, consistent looking total presentation.

To borrow a recent phrase of hch, right now when i look at all the 
ipc related headers it looks like as if a cat barfed all over the 
place ;-)

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/