Hi Chris 

(Sorry about calling you David)

I have just committed a change for the symlink hanging pointer issue you 
raised. It is a bit more verbose than yours but I think it does the right 
thing. I shall be testing it more and running Valgrind testing too.

http://www.aleph1.co.uk/cgi-bin/viewcvs.cgi/yaffs2/yaffs_guts.c?r1=1.95&r2=1.96

-- Charles




On Wednesday 02 December 2009 14:07:44 Charles Manning wrote:
> Thanks David
>
> There is definitely a problem if you unlink a symlink.
>
> I am a bit concerned about the apporach you propose because I don't like
> the idea of using the object after it has been deleted.
>
> Instead I propose to do something like:
> static int yaffs_DeleteSymLink(yaffs_Object * in)
> {
>          YFREE(in->variant.symLinkVariant.alias);
> +      in->variant.symLinkVariant = NULL;
>         return yaffs_DoGenericObjectDeletion(in);
> }
>
> then handling a NULL value properly where need be.
>
> -- Charles
>
> On Wednesday 02 December 2009 12:58:52 Charles Manning wrote:
> > What do you mean by "no longer mountable"?
> >
> > I'll take a look at your analysis to see what's going on.
> >
> > -- Charles
> >
> > On Wednesday 02 December 2009 11:33:52 Chris David wrote:
> > > Hello,
> > >
> > > I've been trying to figure out why sometimes our NAND device becomes
> > > corrupt and is no longer mountable.  The symptom appears to be that in
> > > the yaffs_ScanBackwards function, around line 6719 in yaffs_guts.c,
> > > under the YAFFS_OBJECT_TYPE_SYMLINK case, there is a call:
> > > yaffs_CloneString(oh->alias), and this is returning an empty string.
> > > How could this happen?  Well, I have a possible explanation and a
> > > patch.
> > >
> > > diff -u build_mipsel/linux-2.6.23-msp2/fs/yaffs2/yaffs_guts.c.orig
> > > build_mipsel/linux-2.6.23-msp2/fs/yaffs2/yaffs_guts.c ---
> > > build_mipsel/linux-2.6.23-msp2/fs/yaffs2/yaffs_guts.c.orig  2009-12-01
> > > 14:41:27.000000000 -0800 +++
> > > build_mipsel/linux-2.6.23-msp2/fs/yaffs2/yaffs_guts.c       2009-12-01
> > > 14:42:21.000000000 -0800 @@ -5157,9 +5157,10 @@
> > >
> > >  static int yaffs_DeleteSymLink(yaffs_Object * in)
> > >  {
> > > +       int retv;
> > > +       retv = yaffs_DoGenericObjectDeletion(in);
> > >         YFREE(in->variant.symLinkVariant.alias);
> > > -
> > > -       return yaffs_DoGenericObjectDeletion(in);
> > > +       return retv;
> > >  }
> > >
> > >  static int yaffs_DeleteHardLink(yaffs_Object * in)
> > >
> > > I observed the following by analyzing the code.  There could be a call
> > > stack as follows:
> > >
> > > yaffs_DeleteSymLink(yaffs_Object * in)
> > > yaffs_DoGenericObjectDeletion(in)
> > > yaffs_ChangeObjectName(in, ...
> > > yaffs_UpdateObjectHeader(obj, ...
> > >
> > > which gets to about line 3806 (still yaffs_guts.c), where there is:
> > > yaffs_strncpy(oh->alias,
> > >   in->variant.symLinkVariant.alias,
> > >   YAFFS_MAX_ALIAS_LENGTH);
> > >
> > > well, in->variant.symLinkVariant.alias is already freed at this point.
> > > The above patch would fix this.
> > >
> > > I am not 100% confident in my findings.  Does anyone agree that this
> > > could be a real bug?  I was thinking that if
> > > in->myDev->deletedDir->variantType is set to something other than
> > > YAFFS_OBJECT_TYPE_DIRECTORY, perhaps it isn't.
> > >
> > > Thanks,
> > >
> > > -Chris
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > _______________________________________________
> > > yaffs mailing list
> > > yaffs@lists.aleph1.co.uk
> > > http://lists.aleph1.co.uk/cgi-bin/mailman/listinfo/yaffs
> >
> > _______________________________________________
> > yaffs mailing list
> > yaffs@lists.aleph1.co.uk
> > http://lists.aleph1.co.uk/cgi-bin/mailman/listinfo/yaffs
>
> _______________________________________________
> yaffs mailing list
> yaffs@lists.aleph1.co.uk
> http://lists.aleph1.co.uk/cgi-bin/mailman/listinfo/yaffs