On Sat, Dec 04, 2010 at 02:22:02PM +0300, Andrew Nayenko wrote:
> diff --git a/shlibs/blkid/src/superblocks/nilfs.c b/shlibs/blkid/src/superblocks/nilfs.c
> index bf16918..1f8f3a6 100644
> --- a/shlibs/blkid/src/superblocks/nilfs.c
> +++ b/shlibs/blkid/src/superblocks/nilfs.c
> @@ -84,7 +84,7 @@ static int probe_nilfs2(blkid_probe pr, const struct blkid_idmag *mag)
> if (!sb)
> return -1;
>
> - bytes = le32_to_cpu(sb->s_bytes);
> + bytes = le16_to_cpu(sb->s_bytes);
> crc = crc32(le32_to_cpu(sb->s_crc_seed), (unsigned char *)sb, sumoff);
> crc = crc32(crc, sum, 4);
> crc = crc32(crc, (unsigned char *)sb + sumoff + 4, bytes - sumoff - 4);
Applied, thanks.
> I believe there is a better approach to endianness handling. And compiler is our best
> helper here. We just need to make it generate an error when we try to use a field with
> wrong (or without any) endianness conversion. To achieve this, fields in structures
> should be explicitly declared as little or big endian, e.g.:
> struct sb {
> .....
> xle16_t s_bytes;
> .....
> };
> The xle16_t type should be a complex one, not an integral:
> typedef struct { uint16_t __leu16; } xle16_t;
> Only appropriate conversion functions should know how to convert those types to
> integral ones:
> uint16_t xle16_to_cpu(xle16_t v) { return le16_to_cpu(v.__leu16); }
> xle16_t xcpu_to_le16(uint16_t v) { xle16_t t = {cpu_to_le16(v)}; return t; }
> That's it. Now compiler will enforce us to use the right conversion anytime we
> access a field, i.e. the following statements will fail to compile:
> uint16_t bytes = sb->s_bytes; /* no conversion */
> uint32_t bytes = xle32_to_cpu(sb->s_bytes); /* wrong conversion */
> I use this approach in a real life project (FS driver) and it proved to be a good
> "bugkeeper".
>
> I had a look at shlibs/blkid/src/superblocks/*.c and I think that most probers can
> adopt this approach. But in some cases it can be problematic:
> 1) sometimes fields endianness can vary (e.g. befs, linux_raid) and we do not know
> it at compile time;
> 2) some probers (zfs) prefer to convert fields to native endianness first and then
> just use instead of in-place conversion at any access to the field.
Thanks for the idea, I'll add this to the TODO list for the next
devel phase.
Karel
--
Karel Zak <kzak@xxxxxxxxxx>
http://karelzak.blogspot.com
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
