[U-Boot] [PATCH V2] JFFS2: bug fix for summary support.
Detlev Zundel
dzu at denx.de
Tue Apr 19 17:01:04 CEST 2011
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
More information about the U-Boot
mailing list