Re: [U-Boot] [PATCH v5] Introduced btrfs file-system with btrload command

Hi Adnan,

On Mon, Mar 11, 2013 at 6:18 AM, Adnan Ali adnan.ali@codethink.co.uk 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.

v5: merged with master. v4: btrls command added. v3: patch re-formated. v2: patch error removed.

Signed-off-by: Adnan Ali adnan.ali@codethink.co.uk

Makefile | 1 + common/Makefile | 1 + common/cmd_btr.c | 65 +++ fs/btrfs/Makefile | 51 ++ fs/btrfs/btrfs.c | 1348 ++++++++++++++++++++++++++++++++++++++++++++ fs/btrfs/crc32_c.c | 54 ++ fs/fs.c | 10 + include/btrfs.h | 416 ++++++++++++++ include/config_fallbacks.h | 4 + include/fs.h | 1 + 10 files changed, 1951 insertions(+) create mode 100644 common/cmd_btr.c create mode 100644 fs/btrfs/Makefile create mode 100644 fs/btrfs/btrfs.c create mode 100644 fs/btrfs/crc32_c.c create mode 100644 include/btrfs.h

I have ignored the code before this point in btrfs.c since it is imported and you want to keep the code style as is, but if you don't mind I will make a few comments after that point>

+struct btrfs_info fs;

+/*

• • mount btrfs file-system
• */

+int btrfs_probe(block_dev_desc_t *rbdd, disk_partition_t *info) +{

•    btrfs_block_dev_desc = rbdd;
  

•    part_info = info;
  

•    btr_part_offset = info->start;
  

•   if (btrfs_fs_init(&fs) < 0 ) {
  

•      debug("btrfs probe failed\n");
  

•      return -1;
  

•   }
  

You should use tabs for intend (some of this bit uses spaces, some uses tabs).

•   return 0;
  

+}

+/*

• • Read file data
• */

+int btrfs_read_file(const char *filename, void *buf, int offset, int len) +{

•   int file_len=0;
  

•    int len_read;
  

•    struct com32_filedata filedata;
  

•    int handle;
  

•    if (offset != 0) {
  

•            debug("** Cannot support non-zero offset **\n");
  

•            return -1;
  

•    }
  

•    handle=btrfs_open_file(filename, &filedata);
  

•    if (handle < 0) {
  

•            debug("** File not found %s Invalid handle**\n", filename);
  

•            return -1;
  

•    }
  

•    /*file handle is valid get the size of the file*/
  

•    len = filedata.size;
  

•    if (len == 0)
  

•            len = file_len;
  

•    len_read = getfssec(&filedata, (char *)buf);
  

•    if (len_read != len) {
  

•            debug("** Unable to read file %s **\n", filename);
  

•            return -1;
  

•    }
  

•    return len_read;
  

+}

+/*

• • Show directory entries
• */

+char btrfs_ls(char *dirname) +{

• struct btrfs_dirent *de;
• struct _DIR_ *dir;
• if(*dirname == '/' && *(dirname+1) == 0)

• *dirname = '.';
  

• dir = opendir(dirname);
• if (dir == NULL)

•    return -1;
  

• while ((de = readdir(dir)) != NULL)

•    ;
  

This doesn't seem to print anything.

• return 0;

+}

+/*

• • umount btrfs file-system
• */

+void btrfs_close(void) +{

Remove blank line

+} diff --git a/fs/btrfs/crc32_c.c b/fs/btrfs/crc32_c.c new file mode 100644 index 0000000..78d0447 --- /dev/null +++ b/fs/btrfs/crc32_c.c @@ -0,0 +1,54 @@ +/*

• • 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
• */

Old comment?

+/*

• • Steps through buffer one byte at at time, calculates reflected
• • crc using table.
• */

+#include <linux/stat.h> +#include <command.h> +#include <asm/byteorder.h> +#include <linux/compiler.h> +#include <common.h> +#include <config.h>

+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;
  

+}

+inline void crc32c_init(u32 *crc32c_table, u32 pol)

The 'inline' isn't needed.

+{

•   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++) {
  

Can remove this inner {} since you only have one line of code.

•                   v = (v >> 1) ^ ((v & 1) ? poly : 0);
  

•           }
  

•           crc32c_table[i] = v;
  

•   }
  

+}

I suggest you move your crc_32.c file to lib/ since it might be more generally useful. You should probably include the function prototypes in include/crc.h.

Also you seem to recalculate the crc table on every operation. Is is possible to only do this ones on startup?

Regards, Simon