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

[Simon Glass]

Hi Adnan,

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