From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756945AbYIAQfF (ORCPT ); Mon, 1 Sep 2008 12:35:05 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752579AbYIAQew (ORCPT ); Mon, 1 Sep 2008 12:34:52 -0400 Received: from ey-out-2122.google.com ([74.125.78.25]:57786 "EHLO ey-out-2122.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751825AbYIAQev (ORCPT ); Mon, 1 Sep 2008 12:34:51 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=to:subject:date:user-agent:cc:references:in-reply-to:mime-version :content-type:content-transfer-encoding:content-disposition :message-id:from; b=DtYNhvcHwOqBvQSZPnPZ+W57RUIk9ECxPGql9Hqwb5p3Q765cW9EU9fXOFcvhyy1wS UTasLJImTN+GrcscYf5np90XcrCOv/6QXul5BgmekPfmSbtC3zkMcSY5XgzoSEJfoRMY IkMJ2xXmZ9O8zolL4azbFom44isgg0Jj3qIaA= To: Boaz Harrosh Subject: Re: [PATCH 4/5 ver2] rt2x00: Compiler warning unmasked by fix of BUILD_BUG_ON Date: Mon, 1 Sep 2008 18:34:45 +0200 User-Agent: KMail/1.9.9 Cc: Ingo Molnar , Rusty Russell , "David S. Miller" , "John W. Linville" , Alexey Dobriyan , Andrew Morton , Theodore Tso , Linus Torvalds , Jan Beulich , linux-kernel References: <48BBE77D.7070007@panasas.com> <48BC077A.9000106@panasas.com> In-Reply-To: <48BC077A.9000106@panasas.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200809011834.45856.IvDoorn@gmail.com> From: Ivo van Doorn Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Monday 01 September 2008, Boaz Harrosh wrote: > Ivo Van Doorn wrote: > > On Mon, Sep 1, 2008 at 3:44 PM, Boaz Harrosh wrote: > >> A "Set" of a sign-bit in an "&" operation causes a compiler warning. > >> Make calculations unsigned. > >> > >> [ The warning was masked by the use of (void)() cast in the old > >> BUILD_BUG_ON() ] > >> > >> Signed-off-by: Boaz Harrosh > >> TO: Ivo van Doorn > >> TO: John W. Linville > >> CC: Ingo Molnar > >> CC: Rusty Russell > >> --- > >> drivers/net/wireless/rt2x00/rt2x00reg.h | 4 ++-- > >> 1 files changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/net/wireless/rt2x00/rt2x00reg.h b/drivers/net/wireless/rt2x00/rt2x00reg.h > >> index 7e88ce5..e71b793 100644 > >> --- a/drivers/net/wireless/rt2x00/rt2x00reg.h > >> +++ b/drivers/net/wireless/rt2x00/rt2x00reg.h > >> @@ -136,8 +136,8 @@ struct rt2x00_field32 { > >> */ > >> #define is_power_of_two(x) ( !((x) & ((x)-1)) ) > >> #define low_bit_mask(x) ( ((x)-1) & ~(x) ) > >> -#define is_valid_mask(x) is_power_of_two(1 + (x) + low_bit_mask(x)) > >> - > >> +#define is_valid_mask(x) is_power_of_two(1 + (x) + \ > >> + low_bit_mask((unsigned long)x)) > > > > I know I typed it wrong, but you are missing the unsigned long cast for the > > is_power_of_two argument here (Which could also be done in the > > is_valid_mask() definition). > > > > I thought you suggested that on purpose. Since at the end it is all > one expression, the compiler propagates the cast to all participants. > > Do you want that I send a fix for readability's sake? Yes, thanks. > >> /* > >> * Macro's to find first set bit in a variable. > >> * These macro's behaves the same as the __ffs() function with > >> -- > >> 1.5.6.rc1.5.gadf6 > >> > > Is below what you mean? but if so then perhaps my original one is > clearer. Note that it compiles and works as is. Below patch is good. Your original one would have had the same comment as the original one, where the cast was only done on 1 x instead of both usages. I think the below version is the most clear solution. :) Ivo > --- > diff --git a/drivers/net/wireless/rt2x00/rt2x00reg.h b/drivers/net/wireless/rt2x00/rt2x00reg.h > index 7e88ce5..e71b793 100644 > --- a/drivers/net/wireless/rt2x00/rt2x00reg.h > +++ b/drivers/net/wireless/rt2x00/rt2x00reg.h > @@ -136,8 +136,8 @@ struct rt2x00_field32 { > */ > #define is_power_of_two(x) ( !((x) & ((x)-1)) ) > #define low_bit_mask(x) ( ((x)-1) & ~(x) ) > -#define is_valid_mask(x) is_power_of_two(1 + (x) + low_bit_mask(x)) > - > +#define is_valid_mask(x) is_power_of_two(1LU + (unsigned long)(x) + \ > + low_bit_mask((unsigned long)x)) > /* > * Macro's to find first set bit in a variable. > * These macro's behaves the same as the __ffs() function with > -- 1.5.6.rc1.5.gadf6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ > >