Bug#190732: [PATCH] hurd/libdiskfs/dir-renamed.c

[Marcus Brinkmann]

On Wed, Jul 16, 2003 at 01:26:20AM +0300, Ognyan Kulev wrote:
> On Tue, Jul 15, 2003 at 05:43:24AM -0500, Marcus Brinkmann wrote:
> > 1. What errors do you still expect from the second time diskfs_lookup is
> > invoked with REMOVE?  Is there any error that is allowed at that time?
> > If there is, the change is correct.
> 
> Before the second diskfs_lookup, mutex_unlock is called.  This leaves a 
> little time when someone could remove the node.

The node is unlocked, but the directory is not.  You can not unlink a
file while the directory is locked.  There might be other errors to be
concerned about?
 
> > 2. Is it possible, and according to diskfs.h it should be, to just
> > call dir_lookup with REMOVE only once, at the point you introduced it
> > in your patch, and store the dirstat until it is needed for the actual
> > disks_dirremove call.  This saves one lookup call.  Can you try such a
> > change and look into the code if it is feasible?
> 
> While trying to do that now, I recalled why this cannot be done.  If
> diskfs_lookup is called only once in the beginning, the assertion in 
> ext2fs/dir.c:716 is triggered with the following commands:
> 
> $ mkdir x
> $ mv x y
> $ mv y x
> 
> It seems that dirstat of ext2fs can break when there are other 
> activities in the directory.  It's possible that ext2fs is wrong, but I 
> didn't look further.

Yeah, this seems to be in particular the case if the source and target
directory are the same?  I don't have time to really crunch all that
code until I have figured out what is safe and what is supposed to be
safe.  The easy fix for now, and likely the thing that is really
required, is dropping the dirstat as you did in your patch.
 
> I send a revisited patch that I beleive is easier to read, and it fixes 
> some tiny bugs too.

Thanks.  For now, not being in the position to do much testing and
analysis, I will approve the simple patch and leave the rewritten
version for later.

Thanks,
Marcus