[U-Boot] [PATCH v9] Introduced btrfs file-system with btrload command
Adnan Ali
adnan.ali at codethink.co.uk
Wed Mar 20 17:55:00 CET 2013
On 20/03/13 15:23, Tom Rini wrote:
> On Wed, Mar 20, 2013 at 04:10:05PM +0100, Wolfgang Denk wrote:
>> Dear Adnan Ali,
>>
>> In message <1363789411-9663-1-git-send-email-adnan.ali at codethink.co.uk> you 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.
>>>
>>> v8: patch re-formated.
>>> v7: patch re-formated.
>>> v6: patch re-formated.
>> What exactly is going on here? Why do you have to go through so many
>> iterations just reformatting again and again and again?
> Yes, most of "patch re-formatted" really means "reworked for checkpatch
> problems".
>
>>> +void btrfs_type(char num)
>>> +{
>>> + switch (num) {
>>> + case BTRFS_FILE:
>>> + printf("<FILE> "); break;
>>> + case BTRFS_DIR:
>>> + printf("<DIR> "); break;
>>> + case BTRFS_SYMLNK:
>>> + printf("<SYM> "); break;
>>> + default:
>>> + printf("<UNKNOWN>"); break;
>> Can you please use puts() instead of print() for all output that does
>> not really need any formatting?
> Agreed (and with the other stuff I've snipped too, as those are
> introduced).
Simon was happy with that after i changed all error messages to
debug. But i can change unformatted messages to puts. if everyone
is agrees.
>>> + if (ret < 0)
>>> + low = mid + 1;
>>> + else if (ret > 0)
>>> + high = mid;
>>> + else {
>>> + *slot = mid;
>>> +
>>> + return 0;
>>> + }
>> Is this imported code?
> Yes.
yes
>
>>> + if (__le64_to_cpu(m1->logical) > __le64_to_cpu(m2->logical))
>>> +
>>> + return 1;
>>> +
>>> + if (__le64_to_cpu(m1->logical) < __le64_to_cpu(m2->logical))
>>> +
>>> + return -1;
>> Is this imported code? Otherwise: can we drop these empty lines before
>> the returns?
It wasn't like that simon asked me to add line before return statement.
He was happy with that too. Again i will remove it once everyone agreed
it is the right way.
> This, and the rest are not. But checkpatch.pl doesn't complain about
> them either, annoyingly. Adnan, try doing a diff between the syslinux
> and u-boot files to look for other whitespace oddities that got
> introduced. Thanks. And, thanks for fixing all of the problems
> checkpatch does catch.
>
According to checkpatch there are no whitespaces and errors.
But what exactly are you after. Even though i have removed all
errors and warnings from whole patch.
Thanks
Adnan Ali
More information about the U-Boot
mailing list