2003-07-29 Ognyan Kulev <ogi@fmi.uni-sofia.bg> * dir-renamed.c (checkpath): Redundant assignment is removed. (diskfs_rename_dir): Remove variable BUF. Space for DS is allocated with alloca. When renaming node to itself, goto out instead of repeating code. When checking if TNP is empty directory, assertion "!err" is checked. When maximum link count of TDP is reached, goto out instead of returning immediately. When maximum link count of FNP is reached, goto out instead of repeating code. diskfs_node_update FNP only if diskfs_synchronous. After entering entry TONAME to TDP, set DS to 0. When preparing to remove FROMNAME from FDP, use TMPDS instead of DS. Fix the assertion so that it can handle errors. If error occur, diskfs_drop_dirstat TMPDS before goto out. * dir-rename.c (diskfs_S_dir_rename): Now really serialize directory renaming with RENAMEDIRLOCK. When renaming directory, don't diskfs_node_update and diskfs_file_update because dir-renamed.c::diskfs_rename_dir already do it. When TONAME is .. of filesystem's root, not only set ERR to EINVAL, but return this error. When EXCL is set and target exist, unlock things properly. When FROMNAME is regular file and TONAME is directory, unlock TNP before returning. When maximum link count of FNP is reached and TNP is not NULL, unlock TNP before returning. When updating FNP after incrementing its link count, call diskfs_node_update only if diskfs_synchronous. After TONAME enters TDP, call diskfs_file_update instead of diskfs_node_update. diff -urpN --exclude=build --exclude='*~' --exclude=ChangeLog --exclude=CVS /home/ogi/cvs/hurd/libdiskfs/dir-rename.c hurd/libdiskfs/dir-rename.c --- /home/ogi/cvs/hurd/libdiskfs/dir-rename.c 1997-02-14 03:19:23.000000000 +0200 +++ hurd/libdiskfs/dir-rename.c 2003-07-27 22:05:11.000000000 +0300 @@ -1,5 +1,5 @@ /* libdiskfs implementation of fs.defs: dir_rename - Copyright (C) 1992, 1993, 1994, 1995, 1996, 1997 Free Software Foundation + Copyright (C) 1992,1993,1994,1995,1996,1997,2003 Free Software Foundation This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as @@ -52,7 +52,6 @@ diskfs_S_dir_rename (struct protid *from fdp = fromcred->po->np; tdp = tocred->po->np; - try_again: /* Acquire the source; hold a reference to it. This will prevent anyone from deleting it before we create the new link. */ @@ -66,30 +65,26 @@ diskfs_S_dir_rename (struct protid *from if (S_ISDIR (fnp->dn_stat.st_mode)) { - mutex_unlock (&fnp->lock); - if (!mutex_try_lock (&renamedirlock)) + diskfs_nput (fnp); + + mutex_lock (&renamedirlock); + + /* We now lookup again, but with RENAMEDIRLOCK locked. */ + mutex_lock (&fdp->lock); + err = diskfs_lookup (fdp, fromname, LOOKUP, &fnp, 0, fromcred); + mutex_unlock (&fdp->lock); + if (err == EAGAIN) + err = EINVAL; + if (err) { - diskfs_nrele (fnp); - mutex_lock (&renamedirlock); - goto try_again; + mutex_unlock (&renamedirlock); + return err; } + mutex_unlock (&fnp->lock); + err = diskfs_rename_dir (fdp, fnp, fromname, tdp, toname, fromcred, tocred); - if (diskfs_synchronous) - { - mutex_lock (&fdp->lock); - diskfs_file_update (fdp, 1); - mutex_unlock (&fdp->lock); - - mutex_lock (&fnp->lock); - diskfs_file_update (fnp, 1); - mutex_unlock (&fnp->lock); - - mutex_lock (&tdp->lock); - diskfs_file_update (tdp, 1); - mutex_unlock (&tdp->lock); - } - + diskfs_nrele (fnp); mutex_unlock (&renamedirlock); if (!err) @@ -107,11 +102,14 @@ diskfs_S_dir_rename (struct protid *from err = diskfs_lookup (tdp, toname, RENAME, &tnp, ds, tocred); if (err == EAGAIN) - err = EINVAL; - else if (!err && excl) + err = EINVAL; /* .. of filesystem's root. */ + if (!err && excl) { - err = EEXIST; + diskfs_drop_dirstat (tdp, ds); + diskfs_nrele (fnp); diskfs_nput (tnp); + mutex_unlock (&tdp->lock); + return EEXIST; } if (err && err != ENOENT) { @@ -138,6 +136,7 @@ diskfs_S_dir_rename (struct protid *from { diskfs_drop_dirstat (tdp, ds); diskfs_nrele (fnp); + diskfs_nput (tnp); mutex_unlock (&tdp->lock); return EISDIR; } @@ -149,12 +148,15 @@ diskfs_S_dir_rename (struct protid *from { diskfs_drop_dirstat (tdp, ds); diskfs_nput (fnp); + if (tnp) + diskfs_nput (tnp); mutex_unlock (&tdp->lock); return EMLINK; } fnp->dn_stat.st_nlink++; fnp->dn_set_ctime = 1; - diskfs_node_update (fnp, 1); + if (diskfs_synchronous) + diskfs_node_update (fnp, 1); if (tnp) { @@ -172,7 +174,7 @@ diskfs_S_dir_rename (struct protid *from err = diskfs_direnter (tdp, toname, fnp, ds, tocred); if (diskfs_synchronous) - diskfs_node_update (tdp, 1); + diskfs_file_update (tdp, 1); mutex_unlock (&tdp->lock); mutex_unlock (&fnp->lock); @@ -185,7 +187,7 @@ diskfs_S_dir_rename (struct protid *from /* We now hold no locks */ /* Now we remove the source. Unfortunately, we haven't held - fdp locked (nor could we), so someone else might have already + FDP locked (nor could we), so someone else might have already removed it. */ mutex_lock (&fdp->lock); err = diskfs_lookup (fdp, fromname, REMOVE, &tmpnp, ds, fromcred); diff -urpN --exclude=build --exclude='*~' --exclude=ChangeLog --exclude=CVS /home/ogi/cvs/hurd/libdiskfs/dir-renamed.c hurd/libdiskfs/dir-renamed.c --- /home/ogi/cvs/hurd/libdiskfs/dir-renamed.c 2003-07-29 01:37:24.000000000 +0300 +++ hurd/libdiskfs/dir-renamed.c 2003-07-29 19:37:04.000000000 +0300 @@ -1,5 +1,5 @@ /* - Copyright (C) 1994,95,96,97,98,99,2001,2003 Free Software Foundation, Inc. + Copyright (C) 1994,95,96,97,98,99,2001,03 Free Software Foundation, Inc. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as @@ -32,7 +32,6 @@ checkpath(struct node *source, error_t err; struct node *np; - np = target; for (np = target, err = 0; /* nothing */; /* This special lookup does a diskfs_nput on its first argument @@ -73,9 +72,7 @@ diskfs_rename_dir (struct node *fdp, str { error_t err; struct node *tnp, *tmpnp; - void *buf = alloca (diskfs_dirstat_size); - struct dirstat *ds; - struct dirstat *tmpds; + struct dirstat *ds, *tmpds; mutex_lock (&tdp->lock); diskfs_nref (tdp); /* reference and lock will get consumed by @@ -85,55 +82,60 @@ diskfs_rename_dir (struct node *fdp, str if (err) return err; - /* Now, lock the parent directories. This is legal because tdp is not - a child of fnp (guaranteed by checkpath above). */ + /* Now, lock the parent directories. This is legal because TDP is not + a child of FNP (guaranteed by checkpath above). */ mutex_lock (&fdp->lock); if (fdp != tdp) mutex_lock (&tdp->lock); /* 1: Lookup target; if it exists, make sure it's an empty directory. */ - ds = buf; + ds = alloca (diskfs_dirstat_size); err = diskfs_lookup (tdp, toname, RENAME, &tnp, ds, tocred); assert (err != EAGAIN); /* <-> assert (TONAME != "..") */ - if (tnp == fnp) + /* Renaming node to itself. */ { - diskfs_drop_dirstat (tdp, ds); - diskfs_nput (tnp); - mutex_unlock (&tdp->lock); - if (fdp != tdp) - mutex_unlock (&fdp->lock); - return 0; + err = 0; + fnp = 0; /* Don't unlock the unlocked FNP. */ + goto out; + } + if (err && err != ENOENT) + { + fnp = 0; /* Don't unlock the unlocked FNP. */ + goto out; } - - /* Check permissions to remove FROMNAME and lock FNP. */ - tmpds = alloca (diskfs_dirstat_size); - err = diskfs_lookup (fdp, fromname, REMOVE, &tmpnp, tmpds, fromcred); - assert (!tmpnp || tmpnp == fnp); - if (tmpnp) - diskfs_nrele (tmpnp); - diskfs_drop_dirstat (fdp, tmpds); - if (err) - goto out; if (tnp) { + assert (!err); if (! S_ISDIR(tnp->dn_stat.st_mode)) err = ENOTDIR; else if (!diskfs_dirempty (tnp, tocred)) err = ENOTEMPTY; + if (err) + { + fnp = 0; /* Don't unlock the unlocked FNP. */ + goto out; + } } - if (err && err != ENOENT) + /* 2: Check permissions of the node being moved, and lock FNP. */ + tmpds = alloca (diskfs_dirstat_size); + err = diskfs_lookup (fdp, fromname, REMOVE, &tmpnp, tmpds, fromcred); + assert (!tmpnp || tmpnp == fnp); + if (tmpnp) + diskfs_nrele (tmpnp); + diskfs_drop_dirstat (fdp, tmpds); + if (err) goto out; - /* 2: Set our .. to point to the new parent */ + /* 3: Set our .. to point to the new parent */ if (fdp != tdp) { if (tdp->dn_stat.st_nlink == diskfs_link_max - 1) { err = EMLINK; - return EMLINK; + goto out; } tdp->dn_stat.st_nlink++; tdp->dn_set_ctime = 1; @@ -164,20 +166,17 @@ diskfs_rename_dir (struct node *fdp, str } - /* 3: Increment the link count on the node being moved and rewrite - tdp. */ + /* 4: Increment the link count on the node being moved and rewrite + TDP. */ if (fnp->dn_stat.st_nlink == diskfs_link_max - 1) { - mutex_unlock (&fnp->lock); - diskfs_drop_dirstat (tdp, ds); - mutex_unlock (&tdp->lock); - if (tnp) - diskfs_nput (tnp); - return EMLINK; + err = EMLINK; + goto out; } fnp->dn_stat.st_nlink++; fnp->dn_set_ctime = 1; - diskfs_node_update (fnp, 1); + if (diskfs_synchronous) + diskfs_node_update (fnp, 1); if (tnp) { @@ -195,6 +194,7 @@ diskfs_rename_dir (struct node *fdp, str else { err = diskfs_direnter (tdp, toname, fnp, ds, tocred); + ds = 0; if (diskfs_synchronous) diskfs_file_update (tdp, 1); } @@ -202,18 +202,19 @@ diskfs_rename_dir (struct node *fdp, str if (err) goto out; - /* 4: Remove the entry in fdp. */ - ds = buf; + /* 5: Remove the entry from FDP. */ mutex_unlock (&fnp->lock); - err = diskfs_lookup (fdp, fromname, REMOVE, &tmpnp, ds, fromcred); + err = diskfs_lookup (fdp, fromname, REMOVE, &tmpnp, tmpds, fromcred); assert (!tmpnp || tmpnp == fnp); if (tmpnp) diskfs_nrele (tmpnp); if (err) - goto out; + { + diskfs_drop_dirstat (fdp, tmpds); + goto out; + } - diskfs_dirremove (fdp, fnp, fromname, ds); - ds = 0; + diskfs_dirremove (fdp, fnp, fromname, tmpds); fnp->dn_stat.st_nlink--; fnp->dn_set_ctime = 1; if (diskfs_synchronous)