From: Andi Drebes <lists-receive@programmierforen.de>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: torvalds@linux-foundation.org, linux-fsdevel@vger.kernel.org,
kzak@redhat.com, hch@infradead.org, phillip@lougher.demon.co.uk
Subject: Re: [PATCH 1/2] Make cramfs little endian only
Date: Tue, 4 Dec 2007 21:58:10 +0100 [thread overview]
Message-ID: <200712042158.10988.lists-receive@programmierforen.de> (raw)
In-Reply-To: <20071204121107.e31ddd67.akpm@linux-foundation.org>
> > +/* 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
next prev parent reply other threads:[~2007-12-04 20:58 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-12-04 11:59 [PATCH 0/2] Make cramfs little endian only Andi Drebes
2007-12-04 12:01 ` [PATCH 1/2] " Andi Drebes
2007-12-04 15:34 ` Jörn Engel
2007-12-04 20:37 ` Andi Drebes
2007-12-04 20:58 ` Linus Torvalds
2007-12-04 21:31 ` Andi Drebes
2007-12-04 22:35 ` Linus Torvalds
2007-12-04 22:58 ` Jeff Garzik
2007-12-05 21:57 ` Andi Drebes
2007-12-05 22:21 ` Linus Torvalds
2007-12-05 22:41 ` Jörn Engel
2007-12-06 16:38 ` Andi Drebes
2007-12-06 16:27 ` Andi Drebes
2007-12-06 16:47 ` Jörn Engel
2007-12-06 17:35 ` Linus Torvalds
2007-12-06 17:31 ` Linus Torvalds
2007-12-06 22:37 ` Andi Drebes
2007-12-04 20:11 ` Andrew Morton
2007-12-04 20:58 ` Andi Drebes [this message]
2007-12-04 12:02 ` [PATCH 2/2] Update documentation Andi Drebes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200712042158.10988.lists-receive@programmierforen.de \
--to=lists-receive@programmierforen.de \
--cc=akpm@linux-foundation.org \
--cc=hch@infradead.org \
--cc=kzak@redhat.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=phillip@lougher.demon.co.uk \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.