Subject: Re: [PATCH] ext3: Fix sparse -Wbitwise warnings.
From: "Stephen C. Tweedie" <sct@redhat.com>
To: Alexey Dobriyan <adobriyan@mail.ru>
Cc: Andrew Morton <akpm@osdl.org>, Andreas Dilger <adilger@clusterfs.com>,
       ext3 users list <ext3-users@redhat.com>,
       linux-kernel <linux-kernel@vger.kernel.org>,
       Stephen Tweedie <sct@redhat.com>
In-Reply-To: <200502151246.06598.adobriyan@mail.ru>
References: <200502151246.06598.adobriyan@mail.ru>
Content-Type: text/plain
Content-Transfer-Encoding: 7bit
Message-Id: <1108476729.3363.9.camel@sisko.sctweedie.blueyonder.co.uk>
Mime-Version: 1.0
Date: Tue, 15 Feb 2005 14:12:09 +0000
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1165
Lines: 37

Hi,

On Tue, 2005-02-15 at 10:46, Alexey Dobriyan wrote:

> -			if ((ret = EXT3_HAS_RO_COMPAT_FEATURE(sb,
> -					~EXT3_FEATURE_RO_COMPAT_SUPP))) {
> +			if ((ret = le32_to_cpu(EXT3_HAS_RO_COMPAT_FEATURE(sb,
> +					~EXT3_FEATURE_RO_COMPAT_SUPP)))) {

NAK.

EXT3_HAS_RO_COMPAT_FEATURE returns a boolean value.  It happens to be
implemented internally as 

#define EXT3_HAS_COMPAT_FEATURE(sb,mask)			\
	( EXT3_SB(sb)->s_es->s_feature_compat & cpu_to_le32(mask) )


so the compiler, looking at the preprocessed code, will reasonably
assume it's a genuine little-endian value.  But it's only used as a
boolean, so we shouldn't be requiring the callers to provide an
le32_to_cpu() conversion.

If we want to fix this, let's fix the macros: for example, convert
EXT3_HAS_COMPAT_FEATURE to be

	( le32_to_cpu(EXT3_SB(sb)->s_es->s_feature_compat) & (mask) )

so that we're doing the tests in native CPU endian-ness.

--Stephen

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