Hi Charles, I just realized I sent two messagens (with two patches) yesterday to the wrong email address yesterday. Grumble, grumble.... Anyways, here is one.... On Wednesday 02 December 2009 12:58:52 Charles Manning wrote: > > What do you mean by "no longer mountable"? > here is the console output: # echo 0x2018 > /sys/module/yaffs/parameters/yaffs_traceMask # dmesg -n 8 # mount -t yaffs2 /dev/mtdblock6 /mnt/nand_var/ yaffs: dev is 32505862 name is "mtdblock6" yaffs: passed flags "" yaffs: Attempting MTD mount on 31.6, "mtdblock6" yaffs_ScanBackwards starts intstartblk 1 intendblk 896... Block scanning block 1 state 2 seq 4097 Block scanning block 2 state 2 seq 4098 Block scanning block 3 state 3 seq 0 Block empty Block scanning block 4 state 2 seq 4209 === many similar messages deleted === Block scanning block 896 state 3 seq 0 Block empty 123 blocks to be sorted... ...done 123 blocks to be scanned Allocating from 142 63 Allocating from 142 62 === many similar messages deleted === Allocating from 142 3 Allocating from 142 2 mount: wrong fs type, bad option, bad superblock on /dev/mtdblock6, missing codepage or other error In some cases useful info is found in syslog - try dmesg | tail or so And then the fs is not mounted. So, after analyzing what is happening, I determined that it made it to here: yaffs_guts.c:6720 case YAFFS_OBJECT_TYPE_SYMLINK: if (oh) { in->variant.symLinkVariant.alias = yaffs_CloneString(oh->alias); if (!in->variant.symLinkVariant.alias) alloc_failed = 1; } break; I modified it to print out oh->alias, and also oh->name. oh->alias is the empty string, which causes yaffs_CloneString(oh->alias) to return NULL. oh->name had value "deleted". This what led me to discover the issue. Later On Wednesday 02 December 2009 Charles Manning wrote: > Thanks David > You can call me Chris (my first name). > 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 > Thanks for taking the time to review this. I see your point. I think you mean: + in->variant.symLinkVariant.alias = NULL; ^^^^^ So, I went down this path and below is the patch. (note this one should apply successfully on CVS, unlike my last patch...) I took care to allow mounting a filesystem in the condition my "unmountable filesystem" is currently in. I left this comment in the code in a couple spots to prevent others from changing the critical part. /* Note there used to be a bug where for * deleted symlinks, .alias was copied * from after a free. Therefore, the * value of oh->alias must not be * depended upon for deleted symlinks. */ I'm not sure I like this patch. It will work, but it seems to have added many tests. I have tested this a little. I plan to test it more. One thing I cannot easily test is the YAFFS1 case. yaffs_CloneString() does not differentiate between being passed an empty string to copy, and running out of memory. I was afraid to change it. Perhaps that approach would have resulted in a smaller patch. Is it important that a deleted symlink goes into the dev->deletedDir? --- yaffs2.orig/yaffs_guts.c 2009-11-10 17:40:41.000000000 -0800 +++ yaffs2.fixed7/yaffs_guts.c 2009-12-02 11:23:36.000000000 -0800 @@ -3803,10 +3803,14 @@ /* Do nothing */ break; case YAFFS_OBJECT_TYPE_SYMLINK: - yaffs_strncpy(oh->alias, - in->variant.symLinkVariant.alias, - YAFFS_MAX_ALIAS_LENGTH); - oh->alias[YAFFS_MAX_ALIAS_LENGTH] = 0; + if (! in->variant.symLinkVariant.alias) + oh->alias[0] = 0; + else { + yaffs_strncpy(oh->alias, + in->variant.symLinkVariant.alias, + YAFFS_MAX_ALIAS_LENGTH); + oh->alias[YAFFS_MAX_ALIAS_LENGTH] = 0; + } break; } @@ -5089,6 +5093,8 @@ case YAFFS_OBJECT_TYPE_FILE: return obj->variant.fileVariant.fileSize; case YAFFS_OBJECT_TYPE_SYMLINK: + if (!obj->variant.symLinkVariant.alias) + return 0; return yaffs_strlen(obj->variant.symLinkVariant.alias); default: return 0; @@ -5236,6 +5242,7 @@ static int yaffs_DeleteSymLink(yaffs_Object *in) { YFREE(in->variant.symLinkVariant.alias); + in->variant.symLinkVariant.alias = NULL; return yaffs_DoGenericObjectDeletion(in); } @@ -5974,10 +5981,20 @@ /* Do nothing */ break; case YAFFS_OBJECT_TYPE_SYMLINK: - in->variant.symLinkVariant.alias = - yaffs_CloneString(oh->alias); - if (!in->variant.symLinkVariant.alias) - alloc_failed = 1; + /* Note there used to be a bug where for + * deleted symlinks, .alias was copied + * from after a free. Therefore, the + * value of oh->alias must not be + * depended upon for deleted symlinks. + */ + if (oh->parentObjectId == YAFFS_OBJECTID_DELETED) + in->variant.symLinkVariant.alias = NULL; + else { + in->variant.symLinkVariant.alias = + yaffs_CloneString(oh->alias); + if (!in->variant.symLinkVariant.alias) + alloc_failed = 1; + } break; } @@ -6098,10 +6115,14 @@ yaffs_SetObjectName(in, oh->name); if (in->variantType == YAFFS_OBJECT_TYPE_SYMLINK) { - in->variant.symLinkVariant.alias = - yaffs_CloneString(oh->alias); - if (!in->variant.symLinkVariant.alias) - alloc_failed = 1; /* Not returned to caller */ + if (oh->parentObjectId == YAFFS_OBJECTID_DELETED) + in->variant.symLinkVariant.alias = NULL; + else { + in->variant.symLinkVariant.alias = + yaffs_CloneString(oh->alias); + if (!in->variant.symLinkVariant.alias) + alloc_failed = 1; /* Not reted to be a bug where for + * deleted symlinks, .alias was copied + * from after a free. Therefore, the + * value of oh->alias must not be + * depended upon for deleted symlinks. + */ + + if (oh && oh->parentObjectId == YAFFS_OBJECTID_DELETED) + in->variant.symLinkVariant.alias = NULL; +