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

[Adnan Ali]

On 25/02/13 23:02, Simon Glass wrote:
> Hi,
>
> On Mon, Feb 25, 2013 at 7:04 AM, Tom Rini <trini at 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 at 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 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
>>> + */
>>> +
>>> +/*
>>> + * 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 even
though 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?
> Regards,
> Simon
>
>
>> --
>> Tom