Signed-off-by: Alexey Dobriyan <[email protected]>
---
fs/ext3/resize.c | 20 ++++++++++----------
fs/ext3/super.c | 8 ++++----
include/linux/ext3_fs.h | 2 +-
3 files changed, 15 insertions(+), 15 deletions(-)
Index: linux-warnings/include/linux/ext3_fs.h
===================================================================
--- linux-warnings/include/linux/ext3_fs.h (revision 14)
+++ linux-warnings/include/linux/ext3_fs.h (revision 20)
@@ -450,7 +450,7 @@
*/
__u8 s_prealloc_blocks; /* Nr of blocks to try to preallocate*/
__u8 s_prealloc_dir_blocks; /* Nr to preallocate for dirs */
- __u16 s_reserved_gdt_blocks; /* Per group desc for online growth */
+ __le16 s_reserved_gdt_blocks; /* Per group desc for online growth */
/*
* Journaling support valid if EXT3_FEATURE_COMPAT_HAS_JOURNAL set.
*/
Index: linux-warnings/fs/ext3/super.c
===================================================================
--- linux-warnings/fs/ext3/super.c (revision 14)
+++ linux-warnings/fs/ext3/super.c (revision 20)
@@ -2105,13 +2105,13 @@
ext3_mark_recovery_complete(sb, es);
} else {
- __le32 ret;
- if ((ret = EXT3_HAS_RO_COMPAT_FEATURE(sb,
- ~EXT3_FEATURE_RO_COMPAT_SUPP))) {
+ int ret;
+ if ((ret = le32_to_cpu(EXT3_HAS_RO_COMPAT_FEATURE(sb,
+ ~EXT3_FEATURE_RO_COMPAT_SUPP)))) {
printk(KERN_WARNING "EXT3-fs: %s: couldn't "
"remount RDWR because of unsupported "
"optional features (%x).\n",
- sb->s_id, le32_to_cpu(ret));
+ sb->s_id, ret);
return -EROFS;
}
/*
Index: linux-warnings/fs/ext3/resize.c
===================================================================
--- linux-warnings/fs/ext3/resize.c (revision 14)
+++ linux-warnings/fs/ext3/resize.c (revision 20)
@@ -328,7 +328,7 @@
unsigned five = 5;
unsigned seven = 7;
unsigned grp;
- __u32 *p = (__u32 *)primary->b_data;
+ __le32 *p = (__le32 *)primary->b_data;
int gdbackups = 0;
while ((grp = ext3_list_backups(sb, &three, &five, &seven)) < end) {
@@ -371,7 +371,7 @@
struct buffer_head *dind;
int gdbackups;
struct ext3_iloc iloc;
- __u32 *data;
+ __le32 *data;
int err;
if (test_opt(sb, DEBUG))
@@ -408,7 +408,7 @@
goto exit_bh;
}
- data = (__u32 *)dind->b_data;
+ data = (__le32 *)dind->b_data;
if (le32_to_cpu(data[gdb_num % EXT3_ADDR_PER_BLOCK(sb)]) != gdblock) {
ext3_warning(sb, __FUNCTION__,
"new group %u GDT block %lu not reserved\n",
@@ -510,7 +510,7 @@
struct buffer_head *dind;
struct ext3_iloc iloc;
unsigned long blk;
- __u32 *data, *end;
+ __le32 *data, *end;
int gdbackups = 0;
int res, i;
int err;
@@ -527,15 +527,15 @@
}
blk = EXT3_SB(sb)->s_sbh->b_blocknr + 1 + EXT3_SB(sb)->s_gdb_count;
- data = (__u32 *)dind->b_data + EXT3_SB(sb)->s_gdb_count;
- end = (__u32 *)dind->b_data + EXT3_ADDR_PER_BLOCK(sb);
+ data = (__le32 *)dind->b_data + EXT3_SB(sb)->s_gdb_count;
+ end = (__le32 *)dind->b_data + EXT3_ADDR_PER_BLOCK(sb);
/* Get each reserved primary GDT block and verify it holds backups */
for (res = 0; res < reserved_gdb; res++, blk++) {
if (le32_to_cpu(*data) != blk) {
ext3_warning(sb, __FUNCTION__,
"reserved block %lu not at offset %ld\n",
- blk, (long)(data - (__u32 *)dind->b_data));
+ blk, (long)(data - (__le32 *)dind->b_data));
err = -EINVAL;
goto exit_bh;
}
@@ -550,7 +550,7 @@
goto exit_bh;
}
if (++data >= end)
- data = (__u32 *)dind->b_data;
+ data = (__le32 *)dind->b_data;
}
for (i = 0; i < reserved_gdb; i++) {
@@ -574,7 +574,7 @@
blk = input->group * EXT3_BLOCKS_PER_GROUP(sb);
for (i = 0; i < reserved_gdb; i++) {
int err2;
- data = (__u32 *)primary[i]->b_data;
+ data = (__le32 *)primary[i]->b_data;
/* printk("reserving backup %lu[%u] = %lu\n",
primary[i]->b_blocknr, gdbackups,
blk + primary[i]->b_blocknr); */
@@ -675,7 +675,7 @@
"can't update backup for group %d (err %d), "
"forcing fsck on next reboot\n", group, err);
sbi->s_mount_state &= ~EXT3_VALID_FS;
- sbi->s_es->s_state &= ~cpu_to_le16(EXT3_VALID_FS);
+ sbi->s_es->s_state &= cpu_to_le16(~EXT3_VALID_FS);
mark_buffer_dirty(sbi->s_sbh);
}
}
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
On Tuesday 15 February 2005 16:12, Stephen C. Tweedie wrote:
> 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.
Argh... stupid me. super.c part should be just:
--- linux-2.6.11-rc4/fs/ext3/super.c.orig 2005-02-15 20:01:52.000000000 +0200
+++ linux-2.6.11-rc4/fs/ext3/super.c 2005-02-15 20:02:47.000000000 +0200
@@ -2106,6 +2106,7 @@ static int ext3_remount (struct super_bl
ext3_mark_recovery_complete(sb, es);
} else {
__le32 ret;
+ int ret1;
if ((ret = EXT3_HAS_RO_COMPAT_FEATURE(sb,
~EXT3_FEATURE_RO_COMPAT_SUPP))) {
printk(KERN_WARNING "EXT3-fs: %s: couldn't "
@@ -2122,8 +2123,8 @@ static int ext3_remount (struct super_bl
*/
ext3_clear_journal_err(sb, es);
sbi->s_mount_state = le16_to_cpu(es->s_state);
- if ((ret = ext3_group_extend(sb, es, n_blocks_count)))
- return ret;
+ if ((ret1 = ext3_group_extend(sb, es, n_blocks_count)))
+ return ret1;
if (!ext3_setup_super (sb, es, 0))
sb->s_flags &= ~MS_RDONLY;
}
Stephen C. Tweedie wrote:
> 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) )
Of course that's less efficient though since "mask" is probably constant..
so now the endian conversion changed from compile-time to run-time.
Would something like
( ( EXT3_SB(sb)->s_es->s_feature_compat & cpu_to_le32(mask) ) != 0)
be enough to satisfy sparse?
-Mitch
Hi,
On Tue, 2005-02-15 at 23:29, Mitchell Blank Jr wrote:
> Of course that's less efficient though since "mask" is probably constant..
> so now the endian conversion changed from compile-time to run-time.
>
> Would something like
>
> ( ( EXT3_SB(sb)->s_es->s_feature_compat & cpu_to_le32(mask) ) != 0)
>
> be enough to satisfy sparse?
Yes, but it breaks other places: there are a few places where callers
actually want the real return value from the "&", so that (for example)
they can tell the user which feature failed the compatibility test.
--Stephen
On Feb 15, 2005 15:29 -0800, Mitchell Blank Jr wrote:
> Stephen C. Tweedie wrote:
> > 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) )
>
> Of course that's less efficient though since "mask" is probably constant..
> so now the endian conversion changed from compile-time to run-time.
>
> Would something like
>
> ( ( EXT3_SB(sb)->s_es->s_feature_compat & cpu_to_le32(mask) ) != 0)
>
> be enough to satisfy sparse?
Or we could cast "mask" to the appropriate type (I'm not sure what sparse
uses to determine this).
Cheers, Andreas
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://members.shaw.ca/adilger/ http://members.shaw.ca/golinux/