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.
next prev parent 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