This is the mail archive of the ecos-patches@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: JFFS2 crash fix


On Mon, 2004-08-02 at 13:07, Andrew Lunn wrote:
> On Mon, Jul 19, 2004 at 02:32:05PM +0200, ?yvind Harboe wrote:
> > 
> > 
> > 
> > 
> > -- 
> > ?yvind Harboe
> > http://www.zylin.com
> > 
> 
> > Index: ChangeLog
> > ===================================================================
> > RCS file: /cvs/ecos/ecos/packages/fs/jffs2/current/ChangeLog,v
> > retrieving revision 1.34
> > diff -w -u -r1.34 ChangeLog
> > --- ChangeLog	29 Apr 2004 07:16:10 -0000	1.34
> > +++ ChangeLog	19 Jul 2004 12:30:27 -0000
> > @@ -1,3 +1,9 @@
> > +2004-07-19  Oyvind Harboe <oyvind.harboe@zylin.com>
> > +	
> > +	* src/dir-ecos.c:
> > +	* src/fs-ecos.c:
> > +	Fix problems with errors being propegated as faux-pointers.
> > +	
> >  2004-04-19  Oyvind Harboe <oyvind.harboe@zylin.com>
> >  	
> >  	* src/build.c: JFFS2 can now be used as a write-once, read many mode
> > Index: src/dir-ecos.c
> > ===================================================================
> > RCS file: /cvs/ecos/ecos/packages/fs/jffs2/current/src/dir-ecos.c,v
> > retrieving revision 1.5
> > diff -w -u -r1.5 dir-ecos.c
> > --- src/dir-ecos.c	11 Dec 2003 23:33:54 -0000	1.5
> > +++ src/dir-ecos.c	19 Jul 2004 12:30:27 -0000
> > @@ -48,9 +48,11 @@
> >  	up(&dir_f->sem);
> >  	if (ino) {
> >  		inode = jffs2_iget(dir_i->i_sb, ino);
> > -		if (!inode) {
> > +		if (IS_ERR(inode)) {
> >  			printk("jffs2_iget() failed for ino #%u\n", ino);
> > -			return (ERR_PTR(-EIO));
> > +			// NOTE! inode is *not* a pointer here, but an
> > +			// error code we propagate.
> > +			return inode;
> >  		}
> >  	}
> 
> This is not a complete fix. jffs2_iget can return 0 when malloc
> fails. For this to work correctly you need to modify jffs2_iget so
> that it returns ERR_PTR(-ENOMEM) when out of memory.

Is this a trivial fix, or will it have ramifications for the rest of the
eCos JFFS2 code?

>  Also i don't like
> the comment. The IS_ERR() macro call should be enought to make it
> clear it could be an error code. 

My comment is a bit inapproperiate in that I took the occasion to show
my distaste for multiplexing error codes and pointers into the same 32
bits. Remove.

> 
> >  
> > Index: src/fs-ecos.c
> > ===================================================================
> > RCS file: /cvs/ecos/ecos/packages/fs/jffs2/current/src/fs-ecos.c,v
> > retrieving revision 1.27
> > diff -w -u -r1.27 fs-ecos.c
> > --- src/fs-ecos.c	21 Apr 2004 18:51:21 -0000	1.27
> > +++ src/fs-ecos.c	19 Jul 2004 12:30:28 -0000
> > @@ -302,7 +302,7 @@
> >  	d = jffs2_lookup(dir, name, namelen);
> >  	D2(printf("find_entry got dir = %x\n", d));
> >  
> > -	if (d == NULL)
> > +	if ((d==NULL)||IS_ERR(d))
> >  		return ENOENT;
> 
> You modified the code above so that it passed back the error code and
> now you throw that information away and return ENOENT. Why not return
> the error code?

That does seem like the right thing to do. 

>         Andrew
> 
-- 
Øyvind Harboe
http://www.zylin.com



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]