linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jesper Juhl <juhl-lkml@dif.dk>
To: Andrew Morton <akpm@osdl.org>
Cc: Jesper Juhl <juhl-lkml@dif.dk>,
	davidm@hpl.hp.com, eranian@hpl.hp.com,
	linux-kernel@vger.kernel.org
Subject: Re: [resend][PATCH] avoid signed vs unsigned comparison in efi_range_is_wc()
Date: Thu, 16 Jun 2005 23:02:25 +0200 (CEST)	[thread overview]
Message-ID: <Pine.LNX.4.62.0506162254480.2477@dragon.hyggekrogen.localhost> (raw)
In-Reply-To: <20050616134126.264d6bd5.akpm@osdl.org>

On Thu, 16 Jun 2005, Andrew Morton wrote:

> Jesper Juhl <juhl-lkml@dif.dk> wrote:
> >
> > I send in the patch below a while back but never recieved any response.
> > Now I'm resending it in the hope that it might be added to -mm.
> 
> There are surely many warnings in the tree, hence I'm not really interested
> in patches which only fix `gcc -W' warnings.
> 

Ok, in that case I won't bother you directly with such patches any more 
but instead let them trickle into maintainers trees when they will take 
them.

And yes, I know it's very trivial stuff and it doesn't make much of a 
difference to the "big picture", but my attitude towards that is that no 
issue is too small to be addressed, and since I'm not able to adress many 
of the larger issues I try to address the smaller ones that I'm able to 
handle, and when I run out of those I start nitpicking with the really 
trivial stuff (like gcc -W warnings) - all with the purpose of helping our 
kernel be the very best it can, even if my contribution might be very 
minor in some cases.


> How many are there?
> 

With the .config I use here a regular build gives me 10 warnings. A build 
with gcc -W of the same config gives me 100177 warnings.


> > It looks to me like a significantly large 'len' passed in could cause the 
> > loop to never end. Isn't it safer to make 'i' an unsigned long as well? 
> 
> Nope.  All operations which mix signed and unsigned types promote the
> signed type to unsigned.
> 
Hmm, right, then the only bennefit of the patch as-is is to silence the 
gcc -W warning. But since it can be done 100% safe and the change to use 
an unsigned value for the counter is (at least to me) the logical and 
obviously correct thing to do, I still think the patch has merrit as a 
purely "for pedantic correctness" fix.


-- 
Jesper



  reply	other threads:[~2005-06-16 20:57 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-06-16 20:21 [resend][PATCH] avoid signed vs unsigned comparison in efi_range_is_wc() Jesper Juhl
2005-06-16 20:41 ` Andrew Morton
2005-06-16 21:02   ` Jesper Juhl [this message]
2005-06-16 21:48     ` Arnd Bergmann
2005-06-16 22:24       ` Jesper Juhl
2005-06-16 20:45 ` Richard B. Johnson
2005-06-16 21:07   ` Jesper Juhl
2005-06-16 21:32     ` Arnd Bergmann
2005-06-16 21:01 ` David Mosberger
2005-06-20 17:00   ` Jesse Barnes
2005-06-17  0:25 Arnd Bergmann <arnd@arndb.de>
2005-06-17 17:04 ` Jesper Juhl

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=Pine.LNX.4.62.0506162254480.2477@dragon.hyggekrogen.localhost \
    --to=juhl-lkml@dif.dk \
    --cc=akpm@osdl.org \
    --cc=davidm@hpl.hp.com \
    --cc=eranian@hpl.hp.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;
as well as URLs for NNTP newsgroup(s).