linux-kernel - Re: WARNING: do not add new typedefs

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: <20080102095828.GC23164@shadowen.org>
Date:	Wed, 2 Jan 2008 09:58:28 +0000
From:	Andy Whitcroft <apw@...dowen.org>
To:	Boaz Harrosh <bharrosh@...asas.com>
Cc:	linux-kernel <linux-kernel@...r.kernel.org>,
	Randy Dunlap <rdunlap@...otime.net>,
	Joel Schopp <jschopp@...tin.ibm.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: WARNING: do not add new typedefs - is that for real?

On Tue, Jan 01, 2008 at 06:15:46PM +0200, Boaz Harrosh wrote:
> I have this code:
> 
> <c_code>
> /* 
>  * osd-r10 4.12.5 Data-In and Data-Out buffer offsets
>  * byte offset = mantissa * (2^(exponent+8))
>  */
> typedef __be32 osd_cdb_offset;
> 
> osd_cdb_offset __osd_encode_offset(u64 offset, unsigned *padding,
> 	int min_shift, int max_shift);
> 
> struct osd_attributes_list_mode {
> 	__be32		get_attr_desc_bytes;
> 	osd_cdb_offset	get_attr_desc_offset;
> 
> 	__be32		get_attr_alloc_length;
> 	osd_cdb_offset	get_attr_offset;
> 
> 	__be32		set_attr_bytes;
> 	osd_cdb_offset	set_attr_offset;
> 	__be32 not_used;
> };
> </c_code>
> 
> 
> the osd_cdb_offset above is this special OSD-standard floating-point-like
> special type. It is of size 32 bit in special network order. What should
> I do then:
> 
> __be32 __osd_encode_offset(u64 offset, unsigned *padding,
> 	int min_shift, int max_shift);
> 
> But it is not a __be32. It is this special floating-point-like thingy!!!?
> How was __be32 defined with a #define???!!
> 
> Come on guys, it is not checkpatch.pl place to complain about good language
> constructs that can be misused. This is the maintainers and reviewers
> job to say that a: "typedef struct foo Foo;" is bad practice and we don't
> like it, but it can not be left to a script. "typedef"s should be used
> where they should be used.

It is checkpatch's role to point out things which are likely to be
wrong.  There will always be exceptions.  Lines whihc are much more
readable if they spill over 80 characters, typedefs which do make sense.
atomic_t's for example.  This may well be a valid use of them.  Note
that this is mentioned as a WARNING not an ERROR.  As is stated in the
patch submission notes, you are meant to be comfortable with everything
which checkpatch is still reporting.

checkpatch is a style _guide_, not the be all and end all.  It is meant
to carry a preferred style to try and maintain some consistency kernel
wide.

-apw
--
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/