netdev - Re: [PATCH v2] net: #ifdef inet_bind_bucket::ib

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: <20081111.164554.143409564.davem@davemloft.net>
Date:	Tue, 11 Nov 2008 16:45:54 -0800 (PST)
From:	David Miller <davem@...emloft.net>
To:	adobriyan@...il.com
Cc:	dada1@...mosbay.com, netdev@...r.kernel.org
Subject: Re: [PATCH v2] net: #ifdef inet_bind_bucket::ib_net

From: Alexey Dobriyan <adobriyan@...il.com>
Date: Tue, 11 Nov 2008 14:19:46 +0300

> @@ -35,7 +35,9 @@ struct inet_bind_bucket *inet_bind_bucket_create(struct kmem_cache *cachep,
>  	struct inet_bind_bucket *tb = kmem_cache_alloc(cachep, GFP_ATOMIC);
>  
>  	if (tb != NULL) {
> +#ifdef CONFIG_NET_NS
>  		tb->ib_net       = hold_net(net);
> +#endif
>  		tb->port      = snum;
>  		tb->fastreuse = 0;
>  		INIT_HLIST_HEAD(&tb->owners);

No, this is exactly what we don't want.

If you have to add ifdefs to core C files, you're doing something
wrong.

All the details of ifdef this or ifdef that should be hidden in the
header files.

You cited an example where there are a ton of ifdefs in some header
fule inline function, but that is EXACTLY how this stuff should be
done.  Those header files are where such ugly implementation details
belong.

When people read actual code, they should be concerning themselves
with control flow, what the code is trying to do, etc. rather then
being continually interrupted with ifdef this and ifdef that.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html