[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c
From: |
Ognyan Kulev |
Subject: |
Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c |
Date: |
Tue, 29 Jul 2003 16:46:38 +0300 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030714 Debian/1.4-2 |
Ognyan Kulev wrote:
The patch is ready, but it doesn't revert back st_nlink:s. There are
many small changes so it would be good if someone else take a careful
look too. It features verbose changelog entries too.
The simple patch is committed. The long patch is updated and attached
to this mail. Two bugs since the last long patch are fixed (using nput
instead of nrele and goto out with unlocked FNP).
Regards
--
Ognyan Kulev <ogi@{fmi.uni-sofia.bg,fsa-bg.org}>
7D9F 66E6 68B7 A62B 0FCF EB04 80BF 3A8C A252 9782
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)
- Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c, (continued)
- Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c, Marco Gerards, 2003/07/15
- Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c, Marco Gerards, 2003/07/16
- Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c, Ognyan Kulev, 2003/07/17
- Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c, Marco Gerards, 2003/07/18
- Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c, Ognyan Kulev, 2003/07/21
- Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c, Ognyan Kulev, 2003/07/21
- Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c, Marco Gerards, 2003/07/21
- Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c, Ognyan Kulev, 2003/07/27
- Message not available
- Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c, Ognyan Kulev, 2003/07/28
- Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c,
Ognyan Kulev <=