U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Wolfgang Denk <wd@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v9] Introduced btrfs file-system with btrload command
Date: Wed, 20 Mar 2013 16:10:05 +0100	[thread overview]
Message-ID: <20130320151005.DBD3C20063B@gemini.denx.de> (raw)
In-Reply-To: <1363789411-9663-1-git-send-email-adnan.ali@codethink.co.uk>

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.

  reply	other threads:[~2013-03-20 15:10 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-20 14:23 [U-Boot] [PATCH v9] Introduced btrfs file-system with btrload command Adnan Ali
2013-03-20 15:10 ` Wolfgang Denk [this message]
2013-03-20 15:23   ` Tom Rini
2013-03-20 16:55     ` Adnan Ali
2013-03-20 17:24       ` Tom Rini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130320151005.DBD3C20063B@gemini.denx.de \
    --to=wd@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox