From mboxrd@z Thu Jan  1 00:00:00 1970
From: Wolfgang Denk <wd@denx.de>
Date: Wed, 20 Mar 2013 16:10:05 +0100
Subject: [U-Boot] [PATCH v9] Introduced btrfs file-system with btrload
	command
In-Reply-To: <1363789411-9663-1-git-send-email-adnan.ali@codethink.co.uk>
References: <1363789411-9663-1-git-send-email-adnan.ali@codethink.co.uk>
Message-ID: <20130320151005.DBD3C20063B@gemini.denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Dear Adnan Ali,

In message <1363789411-9663-1-git-send-email-adnan.ali@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?


> +++ b/Makefile
> @@ -257,6 +257,7 @@ endif
>  LIBS-$(CONFIG_OF_EMBED) += dts/libdts.o
>  LIBS-y += arch/$(ARCH)/lib/lib$(ARCH).o
>  LIBS-y += fs/libfs.o \
> +        fs/btrfs/libbtrfs.o \

Indentation by TAB only, please.


> +	if (argc > 5)
> +		strcpy(subvolname, argv[5]);
> +	else
> +		strcpy(subvolname, "");

Please just to:

	subvolname[0] = '\0';

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


> +		if (ret < 0)
> +			low = mid + 1;
> +		else if (ret > 0)
> +			high = mid;
> +		else {
> +			*slot = mid;
> +
> +			return 0;
> +		}

Is this imported code?
CodingStyle says to use braces in all branches if not all branches of
a conditional statement are single line statements.  This should be
fixed globally.

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

> +	if (ret == 0)/* already in map */
> +
> +		return;

Ditto.

> +	if (ret == 0)
> +		slot++;
> +	else if (slot == 0)
> +
> +		return -1;

Ditto. etc.  This should be fixed globally.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
To be is to program.