On Mon, Feb 25, 2013 at 12:24:37PM +0000, Adnan Ali wrote: > Introduces btrfs file-system to read file > from volume/sub-volumes with btrload command. This > implementation has read-only support. > This btrfs implementation is based on syslinux btrfs > code, commit 269ebc845ebc8b46ef4b0be7fa0005c7fdb95b8d. > > Signed-off-by: Adnan Ali <adnan....@codethink.co.uk>
A few things:
- In general in fs/btrfs/btrfs.c I see some coding style problems (lack
of spacing, non-printf's longer than 80-wide). Do these come from
syslinux and thus will make any re-syncs easier?
- It looks like you added support for CONFIG_CMD_FS_GENERIC, if so did
you test that?
- Can you please enable this support code on at least one platform,
preferably the one you've tested and developed with?
[snip]
> diff --git a/fs/fs.c b/fs/fs.c
[snip]
> + //file handle is valid get the size of the file
/* Style comments only */
> + len=filedata.size;
And spacing between variable and assignment.
> @@ -178,7 +248,6 @@ int fs_set_blk_dev(const char *ifname, const char
> *dev_part_str, int fstype)
> for (i = 0; i < ARRAY_SIZE(fstypes); i++) {
> if ((fstype != FS_TYPE_ANY) && (fstype != fstypes[i].fstype))
> continue;
> -
> if (!fstypes[i].probe()) {
> fs_type = fstypes[i].fstype;
> return 0;
[snip]
> @@ -208,7 +280,6 @@ static void fs_close(void)
> int fs_ls(const char *dirname)
> {
> int ret;
> -
> switch (fs_type) {
> case FS_TYPE_FAT:
> ret = fs_ls_fat(dirname);
Unrelated, please drop.
> @@ -237,11 +311,13 @@ int fs_read(const char *filename, ulong addr, int
> offset, int len)
> case FS_TYPE_EXT:
> ret = fs_read_ext(filename, addr, offset, len);
> break;
> + case FS_TYPE_BTR:
> + ret = fs_read_btr(filename, addr, offset, len);
> + break;
> default:
> ret = fs_read_unsupported(filename, addr, offset, len);
> break;
> }
> -
> fs_close();
And unrelated whitespace changes here as well.
> diff --git a/include/btrfs.h b/include/btrfs.h
[snip]
> +/*
> + * Extent structure: contains the mapping of some chunk of a file
> + * that is contiguous on disk.
> + */
> +struct extent {
> + //sector_t pstart; /* Physical start sector */
> + __le64 pstart;
Fix please.
> +/*
> + * Our definition of "not whitespace"
> + */
> +static inline char not_whitespace(char c)
> +{
> + return (unsigned char)c > ' ';
> +}
Can't you just use isspace from <linux/ctypes.h> ?
> diff --git a/include/crc32c.h b/include/crc32c.h
> new file mode 100644
> index 0000000..d04916e
> --- /dev/null
> +++ b/include/crc32c.h
> @@ -0,0 +1,48 @@
> +/*
> + * Copied from Linux kernel crypto/crc32c.c
> + * Copyright (c) 2004 Cisco Systems, Inc.
> + * Copyright (c) 2008 Herbert Xu <herb...@gondor.apana.org.au>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + *
> + */
> +
> +/*
> + * This is the CRC-32C table
> + * Generated with:
> + * width = 32 bits
> + * poly = 0x1EDC6F41
> + * reflect input bytes = true
> + * reflect output bytes = true
> + */
> +
> +/*
> + * Steps through buffer one byte at at time, calculates reflected
> + * crc using table.
> + */
> +
> +static inline u32 crc32c_cal(u32 crc, const char *data, size_t length, u32
> *crc32c_table)
> +{
> + while (length--)
> + crc = crc32c_table[(u8)(crc ^ *data++)] ^ (crc >> 8);
> +
> + return crc;
> +}
> +
> +static inline void crc32c_init(u32 *crc32c_table, u32 pol)
> +{
> + int i, j;
> + u32 v;
> + const u32 poly = pol; /* Bit-reflected CRC32C polynomial */
> +
> + for (i = 0; i < 256; i++) {
> + v = i;
> + for (j = 0; j < 8; j++) {
> + v = (v >> 1) ^ ((v & 1) ? poly : 0);
> + }
> + crc32c_table[i] = v;
> + }
> +}
Simon, since you've just been over all the crc32 recently, do we have
these functions somewhere already that can be easily tapped in to?
Thanks!
--
Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
