From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andi Drebes Subject: Re: [PATCH 1/2] Make cramfs little endian only Date: Tue, 4 Dec 2007 21:58:10 +0100 Message-ID: <200712042158.10988.lists-receive@programmierforen.de> References: <200712041259.52118.lists-receive@programmierforen.de> <200712041301.27285.lists-receive@programmierforen.de> <20071204121107.e31ddd67.akpm@linux-foundation.org> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Cc: torvalds@linux-foundation.org, linux-fsdevel@vger.kernel.org, kzak@redhat.com, hch@infradead.org, phillip@lougher.demon.co.uk To: Andrew Morton Return-path: Received: from server001.webpack.hosteurope.de ([80.237.130.9]:35364 "EHLO server001.webpack.hosteurope.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752701AbXLDU61 (ORCPT ); Tue, 4 Dec 2007 15:58:27 -0500 In-Reply-To: <20071204121107.e31ddd67.akpm@linux-foundation.org> Content-Disposition: inline Sender: linux-fsdevel-owner@vger.kernel.org List-ID: > > +/* Converts a cramfs_info from little endian to big endian. */ > > +static inline void cramfs_convert_inode_letobe(struct cramfs_inode* inode) > > +{ > > + u8* inode_bytes = (u8*)inode; > > + u8 old_nloffs[4]; > > + > > + inode->mode = swab16(inode->mode); > > + inode->uid = swab16(inode->uid); > > + inode->size = (inode_bytes[6] << 16) | (inode_bytes[5] << 8) | (inode_bytes[4]); > > eww. Is there a nicer way of doing that? Yes, I know it's ugly. I searched for a swab24 function. The file fs/jfs/endian24.h defines such a function. Including this file in fs/cramfs/inode.c would be even more ugly than the code above. What about moving this to a header file in include/linux/byteorder? (just a suggestion, perhaps it isn't worth the effort...) > Might be a bit tricky given the weird way in which struct cramfs_inode was > defined. > > > + > > + /* Save the old values of the namelength and the offset */ > > + memcpy(old_nloffs, inode_bytes+8, 4); > > + > > + /* Convert the namelength and the offset */ > > + inode_bytes[8] = ((old_nloffs[0] & 0x3f) << 2) | ((old_nloffs[3] & 0xc0) >> 6); > > + inode_bytes[9] = ((old_nloffs[3] & 0x3f) << 2) | ((old_nloffs[2] & 0xc0) >> 6); > > + inode_bytes[10] = ((old_nloffs[2] & 0x3f) << 2) | ((old_nloffs[1] & 0xc0) >> 6); > > + inode_bytes[11] = ((old_nloffs[1] & 0x3f) << 2) | ((old_nloffs[0] & 0xc0) >> 6); > > +} > > + > > +/* Converts a cramfs superblock from little endian to big endian. */ > > +static inline void cramfs_convert_super_letobe(struct cramfs_super* super) > > +{ > > + super->magic = swab32(super->magic); > > + super->size = swab32(super->size); > > + super->flags = swab32(super->flags); > > + super->future = swab32(super->future); > > + cramfs_convert_info_letobe(&super->fsid); > > + cramfs_convert_inode_letobe(&super->root); > > +} > > These inlines are not sane. Just removing those three takes the sparc64 > fs/cramfs/inode.o from 6856 bytes of text down to 5668, which is rather a > large difference. OK. I'll remove the inline keywords in my copies aswell. Should I prepare a new patch and send it to the mailinglist? > The patch has a number of trivial coding-style errors. > scripts/checkpatch.pl finds them. Oh, I'm sorry. I should have checked that. Andi