[U-Boot] [PATCH] JFFS2: bug fix for summary support.

Detlev Zundel dzu at denx.de
Wed Apr 13 13:23:33 CEST 2011


Hi Leo,

> 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.

Can you give an exmaple how this bug showed?

> 2/ Avoid allocate too big memory if the biggest file in JFFS2 is too
> long. We only allocate one node size for pL->readbuf.
> 3/ Free memory space if we fail to scan the JFFS2.
>
> Signed-off-by: Leo Liu <liucai.lfn at gmail.com>

Please fix the obvious style errors:

  total: 24 errors, 7 warnings, 216 lines checked

  NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
        scripts/cleanfile

> ---
>  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(-)
>
> diff --git a/fs/jffs2/jffs2_1pass.c b/fs/jffs2/jffs2_1pass.c
> index c4f7445..dfb1745 100644
> --- a/fs/jffs2/jffs2_1pass.c
> +++ b/fs/jffs2/jffs2_1pass.c
> @@ -662,7 +662,8 @@ jffs2_free_cache(struct part_info *part)
>  		pL = (struct b_lists *)part->jffs2_priv;
>  		free_nodes(&pL->frag);
>  		free_nodes(&pL->dir);
> -		free(pL->readbuf);
> +		if(pL->readbuf)		
> +			free(pL->readbuf);
>  		free(pL);
>  	}
>  }
> @@ -676,14 +677,19 @@ jffs_init_1pass_list(struct part_info *part)
>  
>  	if (NULL != (part->jffs2_priv = malloc(sizeof(struct b_lists)))) {
>  		pL = (struct b_lists *)part->jffs2_priv;
> -
>  		memset(pL, 0, sizeof(*pL));
> +		
> +		pL->readbuf = malloc(sizeof(union jffs2_node_union));
> +		if(!pL->readbuf) {
> +			printf("jffs_init_1pass_list: malloc failed\n");
> +			return 0;
> +		}
>  #ifdef CONFIG_SYS_JFFS2_SORT_FRAGMENTS
>  		pL->dir.listCompare = compare_dirents;
>  		pL->frag.listCompare = compare_inodes;
>  #endif
>  	}
> -	return 0;
> +	return 1;

Why are you changing return codes here?  I cannot see how this matches
with the patch description.

>  }
>  
>  /* find the inode from the slashless name given a parent */
> @@ -748,8 +754,8 @@ jffs2_1pass_read_inode(struct b_lists *pL, u32 inode, char *dest)
>  
>  			if(dest) {
>  				src = ((uchar *) jNode) + sizeof(struct jffs2_raw_inode);
> -				/* ignore data behind latest known EOF */
> -				if (jNode->offset > totalSize) {
> +				/* ignore data which exceed file length */
> +				if (jNode->offset + jNode->dsize > totalSize) {
>  					put_fl_mem(jNode, pL->readbuf);
>  					continue;
>  				}
> @@ -835,10 +841,10 @@ jffs2_1pass_find_inode(struct b_lists * pL, const char *name, u32 pino)
>  	for(b = pL->dir.listHead; b; b = b->next, counter++) {
>  		jDir = (struct jffs2_raw_dirent *) get_node_mem(b->offset,
>  								pL->readbuf);
> -		if ((pino == jDir->pino) && (len == jDir->nsize) &&
> -		    (jDir->ino) &&	/* 0 for unlink */
> +		if ((pino == jDir->pino) && 
> +			(len == jDir->nsize) &&
>  		    (!strncmp((char *)jDir->name, name, len))) {	/* a match */
> -			if (jDir->version < version) {
> +			if (jDir->version < version) {  /*ignore the old DIRENT*/
>  				put_fl_mem(jDir, pL->readbuf);
>  				continue;
>  			}
> @@ -963,6 +969,13 @@ jffs2_1pass_list_inodes(struct b_lists * pL, u32 pino)
>  			struct jffs2_raw_inode *jNode, *i = NULL;
>  			struct b_node *b2 = pL->frag.listHead;
>  
> +			/*
> +			we compare the DIRENT's ino with the latest DIRENT's ino t determine whether this DIRENT
> +			is the latest. If the DIRENT is not the latest,ignore it.
> +			*/
> +			if(jDir->ino != jffs2_1pass_find_inode(pL, jDir->name, pino))  
> +				continue;
> +			
>  			while (b2) {
>  				jNode = (struct jffs2_raw_inode *)
>  					get_fl_mem(b2->offset, sizeof(ojNode), &ojNode);
> @@ -1448,7 +1461,6 @@ jffs2_1pass_build_lists(struct part_info * part)
>  	u32 counter4 = 0;
>  	u32 counterF = 0;
>  	u32 counterN = 0;
> -	u32 max_totlen = 0;
>  	u32 buf_size = DEFAULT_EMPTY_SCAN_SIZE;
>  	char *buf;
>  
> @@ -1458,9 +1470,16 @@ jffs2_1pass_build_lists(struct part_info * part)
>  	/* lcd_off(); */
>  
>  	/* if we are building a list we need to refresh the cache. */
> -	jffs_init_1pass_list(part);
> -	pL = (struct b_lists *)part->jffs2_priv;
> +	if(! jffs_init_1pass_list(part))
> +		return 0;
> +	
> +	pL = (struct b_lists *)part->jffs2_priv;	
>  	buf = malloc(buf_size);
> +	if (!buf) {
> +		printf("jffs2_1pass_build_lists: malloc failed\n");
> +		return 0;
> +	}
> +	
>  	puts ("Scanning JFFS2 FS:   ");
>  
>  	/* start at the beginning of the partition */

Ah I see, jffs_init_1pass_list can now make the process fail.  This is
worth mentioning in the commit log, i.e. "improve error checking" or
something.  After all, this changes how the code behaves, so it should
be documented.

Cheers
  Detlev

-- 
NAN - No Acronym Neccessary
--
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