Hi Adnan,
On Tue, Feb 26, 2013 at 4:54 AM, Adnan Ali adnan.ali@codethink.co.uk wrote:
On 25/02/13 23:02, Simon Glass wrote:
Hi,
On Mon, Feb 25, 2013 at 7:04 AM, Tom Rini trini@ti.com wrote:
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.ali@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 herbert@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!
Should be - see lib/crc32.c. There is already at least one other copy (ubifs I think) so we should try to avoid adding more.
Maybe the polynomial is different here? But even so it should go with the existing code I think.
I have tried using crc code lib/crc32.c but it always failed eventhough i did change the polynomial but still result is search for file on btrfs fails due to bad crc calculation. I have also enable dynamic table creation but still result is same. so should add my my crc code under ifdef in lib/crc32.c what do you suggest?
I suggest you pull the two functions out and try to see what is different. You might need to define DYNAMIC_CRC_TABLE if the polynomial has changed. 'sandbox' might help here (make sandbox_config) since you can build and run on your linux box.
If you have changed the polynomial and it still doesn't work then something is wrong...
I think adding the code to lib/crc32 with a new CONFIG is OK - but probably better if you put it in a new file like crc32_c.c or similar. Hopefully there is some common element to the algorithms?
Regards, Simon
Regards, Simon
-- Tom