linux-kernel - Re: [PATCH 1/6] Werror: Fix casting wrong size integer to pointer

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: <alpine.LFD.1.10.0805291123320.2958@woody.linux-foundation.org>
Date:	Thu, 29 May 2008 11:29:41 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	David Howells <dhowells@...hat.com>
cc:	akpm@...ux-foundation.org, bunk@...nel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/6] Werror: Fix casting wrong size integer to pointer

On Thu, 29 May 2008, David Howells wrote:
>
> Fix casting wrong size integer to pointer.  Smaller integers must be cast to
> unsigned long then cast to a pointer.

I do not want to apply patches that just hide compiler warnings.

Quite frankly, if the code looks worse (and it does), and the warning is 
valid (and in two out of three cases it *was*), then the patch that 
removes the warning is MUCH MUCH WORSE than the warning ever was!

So in this case, the "sis_mm.c" change *may* be fine. I say *may*, because 
quite frankly, I don't think it really is. The cast looks like it may be 
at the wrogn point, and the code really looks like crap. In particular, 
notice how the "sis_sman_mm_allocate()" and "sis_sman_mm_free()" (and 
the _offset() function) use casts on different sides of the '~' operator.

I think that thing should be cleaned up to actually have a function to 
turn the offset into a pointer and back. Because right now it looks buggy.

The two changes to drivers/message/i2o/i2o_config.c are utter shit. That 
warning should *stay*, because it damn well is correct. The code simply 
doesn't work right on a 64-bit architecture, and even has a comment to 
that effect.

Don't send patches to remove warnings. Send patches to FIX PROBLEMS. If 
the warning goes away because the _problem_ went away, it's a good patch. 
Otherwise the patch is just a total piece of crap.

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