[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