From: Andi Drebes <lists-receive@programmierforen.de>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "Jörn Engel" <joern@logfs.org>,
linux-fsdevel@vger.kernel.org, "Karel Zak" <kzak@redhat.com>,
"Andrew Morton" <akpm@linux-foundation.org>,
"Christoph Hellwig" <hch@infradead.org>,
"Phillip Lougher" <phillip@lougher.demon.co.uk>
Subject: Re: [PATCH 1/2] Make cramfs little endian only
Date: Tue, 4 Dec 2007 22:31:54 +0100 [thread overview]
Message-ID: <200712042231.54640.lists-receive@programmierforen.de> (raw)
In-Reply-To: <alpine.LFD.0.9999.0712041250280.32379@woody.linux-foundation.org>
> > Sure. This saves some definitions (and lines of code)...
> > Here's the new patch (tested on the same machines mentioned in the first message).
> > I tried to move as many lines as possible out of the endian dependent section.
>
> This really is the totally wrong way to do this.
>
> You should *not* convert inodes to CPU-endian mode at all. You should
> *keep* them in the native mode, and then just use "le32_to_cpu()" when
> actually using them.
OK. I'll prepare a new patch and send it to the list (not today,
it's already too late in the evening here).
> Basically, if you ever have a
>
> #ifdef __BIG_ENDIAN
>
> or similar in the source code, you're simply doing something wrong.
Perhaps I'm missing somehting, but I think for cramfs, unfortunately,
there has to be this statement. The bitfields in the cramfs_inode structure
cause some problems. You can't simply call le32_to_cpu() on them.
Especially the namelength and offset fields are weird. There has to be at least
one routine for each kind of machine that converts those values (or not -- depending
ont the machine's endianness).
> Btw, sparse can be a big help for things like this, by just marking the
> actual disk data structures as being the right type (ie "__le32" and
> friends), and then you can verify that any users will use "le32_to_cpu()"
> as required, because sparse will warn about bad endianness.
Ok, thanks for your advice. But what about the problems mentioned above?
Andi
next prev parent reply other threads:[~2007-12-04 21:32 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 [this message]
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
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=200712042231.54640.lists-receive@programmierforen.de \
--to=lists-receive@programmierforen.de \
--cc=akpm@linux-foundation.org \
--cc=hch@infradead.org \
--cc=joern@logfs.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).