[PATCH] e2fsprogs: Fix tst_extents output on bigendian machine.

[PATCH] e2fsprogs: Fix tst_extents output on bigendian machine.

[Aneesh Kumar K.V]

Add type __le16, __le32, and __le64 to indicate that
the variables need to be byteswaped when using.
Also fix the header priting on powerpc machine.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 lib/ext2fs/ext3_extents.h |   30 +++++++++++++++++-------------
 lib/ext2fs/extent.c       |   20 +++++++++++++++-----
 2 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/lib/ext2fs/ext3_extents.h b/lib/ext2fs/ext3_extents.h
index 3b373c7..4018bd4 100644
--- a/lib/ext2fs/ext3_extents.h
+++ b/lib/ext2fs/ext3_extents.h
@@ -19,6 +19,10 @@
 #ifndef _LINUX_EXT3_EXTENTS
 #define _LINUX_EXT3_EXTENTS
 
+typedef __u16	__le16;
+typedef	__u32	__le32;
+typedef	__u64	__le64;
+
 /*
  * ext3_inode has i_block array (total 60 bytes)
  * first 4 bytes are used to store:
@@ -31,10 +35,10 @@
  * it's used at the bottom of the tree
  */
 struct ext3_extent {
-	__u32	ee_block;	/* first logical block extent covers */
-	__u16	ee_len;		/* number of blocks covered by extent */
-	__u16	ee_start_hi;	/* high 16 bits of physical block */
-	__u32	ee_start;	/* low 32 bigs of physical block */
+	__le32	ee_block;	/* first logical block extent covers */
+	__le16	ee_len;		/* number of blocks covered by extent */
+	__le16	ee_start_hi;	/* high 16 bits of physical block */
+	__le32	ee_start;	/* low 32 bigs of physical block */
 };
 
 /*
@@ -42,22 +46,22 @@ struct ext3_extent {
  * it's used at all the levels, but the bottom
  */
 struct ext3_extent_idx {
-	__u32	ei_block;	/* index covers logical blocks from 'block' */
-	__u32	ei_leaf;	/* pointer to the physical block of the next *
+	__le32	ei_block;	/* index covers logical blocks from 'block' */
+	__le32	ei_leaf;	/* pointer to the physical block of the next *
 				 * level. leaf or next index could bet here */
-	__u16	ei_leaf_hi;	/* high 16 bits of physical block */
-	__u16	ei_unused;
+	__le16	ei_leaf_hi;	/* high 16 bits of physical block */
+	__le16	ei_unused;
 };
 
 /*
  * each block (leaves and indexes), even inode-stored has header
  */
 struct ext3_extent_header {
-	__u16	eh_magic;	/* probably will support different formats */
-	__u16	eh_entries;	/* number of valid entries */
-	__u16	eh_max;		/* capacity of store in entries */
-	__u16	eh_depth;	/* has tree real underlaying blocks? */
-	__u32	eh_generation;	/* generation of the tree */
+	__le16	eh_magic;	/* probably will support different formats */
+	__le16	eh_entries;	/* number of valid entries */
+	__le16	eh_max;		/* capacity of store in entries */
+	__le16	eh_depth;	/* has tree real underlaying blocks? */
+	__le32	eh_generation;	/* generation of the tree */
 };
 
 #define EXT3_EXT_MAGIC		0xf30a
diff --git a/lib/ext2fs/extent.c b/lib/ext2fs/extent.c
index a409252..44d7d9b 100644
--- a/lib/ext2fs/extent.c
+++ b/lib/ext2fs/extent.c
@@ -73,21 +73,31 @@ struct ext2_extent_path {
 static void dbg_show_header(struct ext3_extent_header *eh)
 {
 	printf("header: magic=%x entries=%u max=%u depth=%u generation=%u\n",
-	       eh->eh_magic, eh->eh_entries, eh->eh_max, eh->eh_depth,
-	       eh->eh_generation);
+			ext2fs_le16_to_cpu(eh->eh_magic),
+			ext2fs_le16_to_cpu(eh->eh_entries),
+			ext2fs_le16_to_cpu(eh->eh_max),
+			ext2fs_le16_to_cpu(eh->eh_depth),
+			ext2fs_le32_to_cpu(eh->eh_generation));
 }
 
 static void dbg_show_index(struct ext3_extent_idx *ix)
 {
 	printf("index: block=%u leaf=%u leaf_hi=%u unused=%u\n",
-	       ix->ei_block, ix->ei_leaf, ix->ei_leaf_hi, ix->ei_unused);
+			ext2fs_le32_to_cpu(ix->ei_block),
+			ext2fs_le32_to_cpu(ix->ei_leaf),
+			ext2fs_le16_to_cpu(ix->ei_leaf_hi),
+			ext2fs_le16_to_cpu(ix->ei_unused));
 }
 
 static void dbg_show_extent(struct ext3_extent *ex)
 {
 	printf("extent: block=%u-%u len=%u start=%u start_hi=%u\n",
-	       ex->ee_block, ex->ee_block + ex->ee_len - 1,
-	       ex->ee_len, ex->ee_start, ex->ee_start_hi);
+			ext2fs_le32_to_cpu(ex->ee_block),
+			ext2fs_le32_to_cpu(ex->ee_block) +
+			ext2fs_le16_to_cpu(ex->ee_len) - 1,
+			ext2fs_le16_to_cpu(ex->ee_len),
+			ext2fs_le32_to_cpu(ex->ee_start),
+			ext2fs_le16_to_cpu(ex->ee_start_hi));
 }
 
 static void dbg_print_extent(char *desc, struct ext2fs_extent *extent)
-- 
1.5.6.3.439.g1e10.dirty

Re: [PATCH] e2fsprogs: Fix tst_extents output on bigendian machine.

[Theodore Tso]

On Fri, Jul 18, 2008 at 05:35:40PM +0530, Aneesh Kumar K.V wrote:
> Add type __le16, __le32, and __le64 to indicate that
> the variables need to be byteswaped when using.

I took out the __le16 types.  The artificial types don't really buy us
much (a lot of work would be needed before we could use sparse on
e2fsprogs) and for now just makes it harder to read the header file.
I removed the __le types deliberately.

I've applied the header printing portion of the patch.

     	     	 		 	    - Ted

Re: [PATCH] e2fsprogs: Fix tst_extents output on bigendian machine.

[Aneesh Kumar K.V]

On Fri, Jul 18, 2008 at 09:19:21PM -0400, Theodore Tso wrote:
> On Fri, Jul 18, 2008 at 05:35:40PM +0530, Aneesh Kumar K.V wrote:
> > Add type __le16, __le32, and __le64 to indicate that
> > the variables need to be byteswaped when using.
> 
> I took out the __le16 types.  The artificial types don't really buy us
> much (a lot of work would be needed before we could use sparse on
> e2fsprogs) and for now just makes it harder to read the header file.
> I removed the __le types deliberately.
> 

The reason for me to use __le* types was to explicitly document that
these variables need to be accessed through the conversion APIs. In this
case I had to search rest of the code to figure out whether all of them
need to be accessed via the conversion APIs or not. Getting sparse to
run on e2fsprogs can be a long term goal. But more important is to
indicate how these variables need to be accessed in the program.


-aneesh

Re: [PATCH] e2fsprogs: Fix tst_extents output on bigendian machine.

[Andreas Dilger]

On Jul 18, 2008  17:35 +0530, Aneesh Kumar wrote:
>  static void dbg_show_header(struct ext3_extent_header *eh)
>  {
>  	printf("header: magic=%x entries=%u max=%u depth=%u generation=%u\n",
> -	       eh->eh_magic, eh->eh_entries, eh->eh_max, eh->eh_depth,
> -	       eh->eh_generation);
> +			ext2fs_le16_to_cpu(eh->eh_magic),
> +			ext2fs_le16_to_cpu(eh->eh_entries),
> +			ext2fs_le16_to_cpu(eh->eh_max),
> +			ext2fs_le16_to_cpu(eh->eh_depth),
> +			ext2fs_le32_to_cpu(eh->eh_generation));

Please fix indenting to align this with the '(' on the preceeding line.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

Re: [PATCH] e2fsprogs: Fix tst_extents output on bigendian machine.

[Aneesh Kumar K.V]

On Sun, Jul 20, 2008 at 11:07:55PM -0600, Andreas Dilger wrote:
> On Jul 18, 2008  17:35 +0530, Aneesh Kumar wrote:
> >  static void dbg_show_header(struct ext3_extent_header *eh)
> >  {
> >  	printf("header: magic=%x entries=%u max=%u depth=%u generation=%u\n",
> > -	       eh->eh_magic, eh->eh_entries, eh->eh_max, eh->eh_depth,
> > -	       eh->eh_generation);
> > +			ext2fs_le16_to_cpu(eh->eh_magic),
> > +			ext2fs_le16_to_cpu(eh->eh_entries),
> > +			ext2fs_le16_to_cpu(eh->eh_max),
> > +			ext2fs_le16_to_cpu(eh->eh_depth),
> > +			ext2fs_le32_to_cpu(eh->eh_generation));
> 
> Please fix indenting to align this with the '(' on the preceeding line.
> 

I am always confused what is the suggested coding guideline here. I
always use TAB instead of space and use TAB aligned offsets for the
continuation line.


-aneesh