[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