mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: David Howells <dhowells@redhat.com>
Cc: akpm@linux-foundation.org, bunk@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/6] Werror: Fix casting wrong size integer to pointer
Date: Thu, 29 May 2008 11:29:41 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.1.10.0805291123320.2958@woody.linux-foundation.org> (raw)
In-Reply-To: <20080529181726.5396.79336.stgit@warthog.procyon.org.uk>



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

  parent reply	other threads:[~2008-05-29 18:34 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-29 18:17 David Howells
2008-05-29 18:17 ` [PATCH 2/6] Werror: Hide epca_setup() as it's unused David Howells
2008-05-29 18:35   ` Alan Cox
2008-05-29 18:17 ` [PATCH 3/6] Werror: Remove iiEllisCleanup() " David Howells
2008-05-29 18:28   ` Adrian Bunk
2008-05-29 18:35   ` Alan Cox
2008-05-29 19:07   ` David Howells
2008-05-29 18:17 ` [PATCH 4/6] Werror: Hide warnings on static module device tables David Howells
2008-05-29 19:24   ` Adrian Bunk
2008-05-29 19:41   ` David Howells
2008-05-29 19:50     ` Adrian Bunk
2008-05-29 18:17 ` [PATCH 5/6] Werror: Remove the packed attribute from PofTimStamp_tag in the hysdn driver David Howells
2008-05-29 19:24   ` Andrew Morton
2008-05-29 18:17 ` [PATCH 6/6] Werror: Provide a configuration option to enable -Werror David Howells
2008-05-29 18:30   ` Linus Torvalds
2008-05-29 18:29 ` Linus Torvalds [this message]
2008-05-29 18:48 ` [PATCH 1/6] Werror: Fix casting wrong size integer to pointer David Howells
2008-05-29 19:19 ` Andrew Morton
2008-05-29 19:25 ` David Howells
2008-05-29 20:08   ` Andrew Morton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.1.10.0805291123320.2958@woody.linux-foundation.org \
    --to=torvalds@linux-foundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=bunk@kernel.org \
    --cc=dhowells@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®