Re: [PATCH][MIPS][2/7] AR7: mtd partition map

["Jörn Engel" <joern@logfs.org>]

On Thu, 20 September 2007 21:35:49 +0200, Christoph Hellwig wrote:
> On Thu, Sep 20, 2007 at 09:29:11PM +0200, Matteo Croce wrote:
> > +#ifdef CONFIG_CPU_LITTLE_ENDIAN
> > +#define LOADER_MAGIC1	0xfeedfa42
> > +#define LOADER_MAGIC2	0xfeed1281
> > +#else
> > +#define LOADER_MAGIC1	0x42faedfe
> > +#define LOADER_MAGIC2	0x8112edfe
> > +#endif
> 
> Please keep only one defintion and use le/be32_to_cpu on it.
> 
> > +struct ar7_bin_rec {
> > +	unsigned int checksum;
> > +	unsigned int length;
> > +	unsigned int address;
> > +};
> 
> Wich means you'd need an endianess annotation here.  What about the
> length and address fields, are they always native-endian unlike
> the checksum field or will the need to be byte-swapped aswell?

<slightly off-topic, feel free to skip>
If this is indeed the squashfs magic, le/be32_to_cpu won't help.
Squashfs can have either endianness, tries to detect the one actually
used by checking either magic and sets a flag in the superblock.
Afterwards every single access checks the flag and conditionally swaps
fields around or not.

If squashfs had a fixed endianness, quite a lot of this logic could get
removed and both source and object size would shrink.  Some two years
after requesting this for the first time, I'm thinking about just doing
it myself.  If I find a sponsor who pays me for it, I might even do it
soon.
</offtopic>


I don't really understand why the squashfs magic number should be used
in this code at all.  It may have set a bad example, though.  In general
you should decide on a fixed endianness (1) and use the beXX_to_cpu
macros when accessing data unless you have a very good reason to do
otherwise.

1) Big endian is my preferred choice because it is easy to read in a
hexdump and the opposite of my notebook.  Being forced to do endian
conversions during development/testing helps to find problems early.

Jörn

-- 
Joern's library part 13:
http://www.chip-architect.com/