Re: Hurd bug in libdiskfs (patch included)

[Abstract]

> It seems to me like this error should be caught in step 4, and indeed
> it is, but sense the diskfs_looup returns an error, it never fills
> tmpnp, so the immediately following assert gets signalled.  It should
> be satisfactory to rearrange the code.

It seems illogical to me to modify the filesystem (changing the
directory in question's `..' and adding it to the target) before finding
out if the operation was permitted in the first place. And even if you do
rearrange the code, you'd still need to do one of the three changes first,
and each one leaves one of the places where permission may be denied
unchecked (that is, the source dir, the source dir's parent and the dest
dir -- ignoring for a second the possible presence of an empty target
already existing). As far as I can see, whatever you do, you need to check
the permissions on one of the nodes before you start, or do some ugly
backtracking-type fixups to the filesystem afterwards.

However, since dir_lookup(...) has to be called on tdp anyway, I suppose
it would be workable to order them so that the first change to the
filesystem alters fdp and fnp. But that change is removing fnp from the
directory, leaving it with a reference count of 1, which I'd imagine might
automatically delete it (though I suppose you'd know much better than me).
Since I assumed it would, I decided against that fix.

> First, Richard, can you confirm *which* assert is failing for you?

OK. It's the assert (tmpnp == fnp); on line 202 of the unpatched file.
Sorry for not saying that in my original bug report; I shall try to be
much clearer next time =)

Richard