From: Detlev Zundel <dzu@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH V2] JFFS2: bug fix for summary support.
Date: Tue, 19 Apr 2011 17:01:04 +0200 [thread overview]
Message-ID: <m2vcyai8un.fsf@ohwell.denx.de> (raw)
In-Reply-To: <001201cbfca4$2fc3fa80$6401a8c0@LENOVOE5CA6843> (Baidu Liu's message of "Sun, 17 Apr 2011 10:07:02 +0800")
Hi Baidu,
> This patch fixes some issues with JFFS2 summary support in U-Boot.
> 1/ Bug fix for summary support: we need to get the latest DIRENT.
> For example, if you create a file in linux jffs2 which config summary
> support, then you delete the file , you will not see the file in
> linux jffs2. But you can also see this file in uboot after you reset
> the system. That is because all the nodes in jffs2 which config summary
> will not be marked as obsolete. The deleted file's DIRENT node will be
> seen in uboot. So what we need to do is to get the latest DIRENT whose
> ino in DIRENT is 0. Than we will not see this file in uboot which is
> what we want.
> 2/ Add CONFIG_SYS_JFFS2_SORT_FRAGMENTS definition,if we config jffs2
> summary in uboot.
> 3/ Avoid allocate too big memory if the biggest file in JFFS2 is too
> long. We only allocate one node size for pL->readbuf.
> For example, if the biggest file in the jffs2 is 15MB in the 16MB jffs2
> system. Our previous code will allocate a buffer(pL->readbuf) as 15MB
> length.
> 4/ Improve error checking in the scanning.
I missed saying this explicitely in my first review that we should
strive to isolate changes into single commits. As you have added even
more changes into a single commit this has now become somewhat
untolerable. Please split into individual functional changes (git add
-i can work wonders here), i.e. the list items in the changelog should
become individual commits.
This also includes the added WATCHDOG_RESET not mentioned in the
changelog at all ;)
> Changes for v2:
> - Add detail description for the patch.
Are you sure you only changed the description? The diffstat from this
and the last time disagree - this time:
> ---
> fs/jffs2/jffs2_1pass.c | 60 +++++++++++++++++++++++++-----------------
> fs/jffs2/jffs2_nand_1pass.c | 28 +++++++++++++++-----
> include/jffs2/jffs2.h | 10 +++++++
> 3 files changed, 67 insertions(+), 31 deletions(-)
Last time:
> fs/jffs2/jffs2_1pass.c | 53 +++++++++++++++++++++++++-----------------
> fs/jffs2/jffs2_nand_1pass.c | 24 ++++++++++++++-----
> include/jffs2/jffs2.h | 11 +++++++++
> 3 files changed, 60 insertions(+), 28 deletions(-)
Looking over the changes, I do _see_ changes in code, so you should tell
us about them.
> [...]
> diff --git a/include/jffs2/jffs2.h b/include/jffs2/jffs2.h
> index 651f94c..5b006c0 100644
> --- a/include/jffs2/jffs2.h
> +++ b/include/jffs2/jffs2.h
> @@ -41,6 +41,16 @@
> #include <asm/types.h>
> #include <jffs2/load_kernel.h>
>
> +#ifdef CONFIG_JFFS2_SUMMARY
> +#ifndef CONFIG_SYS_JFFS2_SORT_FRAGMENTS
> +/*
> +we should define CONFIG_SYS_JFFS2_SORT_FRAGMENTS,if
> +CONFIG_JFFS2_SUMMARY is enabled.
> +*/
> +#define CONFIG_SYS_JFFS2_SORT_FRAGMENTS
> +#endif
> +#endif
> +
> #define JFFS2_SUPER_MAGIC 0x72b6
>
> /* Values we may expect to find in the 'magic' field */
I liked the previous version better:
> diff --git a/include/jffs2/jffs2.h b/include/jffs2/jffs2.h
> index 651f94c..c01a76e 100644
> --- a/include/jffs2/jffs2.h
> +++ b/include/jffs2/jffs2.h
> @@ -41,6 +41,17 @@
> #include <asm/types.h>
> #include <jffs2/load_kernel.h>
>
> +#ifdef CONFIG_JFFS2_SUMMARY
> +#ifndef CONFIG_SYS_JFFS2_SORT_FRAGMENTS
> +/*
> + * if we define summary in jffs2, we also need to define
> + * CONFIG_SYS_JFFS2_SORT_FRAGMENTS. If not, the data in latest inode may be
> + * overwritten by the old one.
> +*/
> +#error "need to define CONFIG_SYS_JFFS2_SORT_FRAGMENTS,if summary is enabled"
> +#endif
> +#endif
> +
> #define JFFS2_SUPER_MAGIC 0x72b6
Why did you change this to the worse?
Cheers
Detlev
--
Q: Why did the chicken cross the Moebius strip?
A: To get to the other ... er, um ...
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de
next prev parent reply other threads:[~2011-04-19 15:01 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-17 2:07 [U-Boot] [PATCH V2] JFFS2: bug fix for summary support Baidu Liu
2011-04-19 15:01 ` Detlev Zundel [this message]
2011-04-24 3:45 ` Baidu Liu
2011-04-27 9:45 ` Detlev Zundel
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=m2vcyai8un.fsf@ohwell.denx.de \
--to=dzu@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