[U-Boot] [PATCH] JFFS2: bug fix for summary support.
Baidu Liu
liucai.lfn at gmail.com
Wed Apr 13 15:19:35 CEST 2011
Hi, Detlev:
2011/4/13 Detlev Zundel <dzu at denx.de>:
> 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?
>
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 support
will not be marked as obsolete. The delete 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. So we will not see this file in uboot which is
what we want.
>> 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
>
I will send a new patch, thanks
>> ---
>> 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.
we may failed when we do malloc, so we need to check the return value
of the malloc.
>> }
>>
>> /* 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 */
actually , this is the key point of this patch
>> + 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;
actually , this is the key point of this patch
>> +
>> 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.
>
I will do it in the next patch, thanks
> 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