Re: [PATCH] dcache: trivial comment fix

[Neil Brown]

On Monday September 10, bfields@xxxxxxxxxxxx wrote:
> On Mon, Sep 10, 2007 at 02:46:32PM -0400, J. Bruce Fields wrote:
> >   * This forceful removal will result in ugly /proc output if
> >   * somebody holds a file open that got deleted due to a rename.
> >   * We could be nicer about the deleted file, and let it show
> > - * up under the name it got deleted rather than the name that
> > - * deleted it.
> > + * up under the name it had before it was deleted rather than
> > + * under the original name of the file that was moved on top of it.
> 
> By the way, on further examination of the code it doesn't actually do
> what's described in the case where the target name is large and the
> moved-from name is small.  Instead, it reports random garbage (usually
> part of a name left over from some other dentry?) as far as I can tell:
> 
> from switch_names():
> 
> 
> 	if (dname_external(target)) {
>                 if (dname_external(dentry)) {
> 			...
>                 } else {
>                         /*
>                          * dentry:internal, target:external.  Steal target's
>                          * storage and make target internal.
>                          */
>                         dentry->d_name.name = target->d_name.name;
>                         target->d_name.name = target->d_iname;
> 
> ... but target->d_iname could have anything in it, right?

Right, but not relevant.
The name "switch_names" is somewhat misleading.  It is really
"copyname" or similar.  From the comment at the top:

 * When switching names, the actual string doesn't strictly have to
 * be preserved in the target - because we're dropping the target
 * anyway. As such, we can just do a simple memcpy() to copy over
 * the new name before we switch.

so the apparent name of 'target' after the 'swap' is not important.

The purpose of the assignment
                         target->d_name.name = target->d_iname;
is to make "dname_external(target)" false, that making "target
internal" as the comment says.

static inline int dname_external(struct dentry *dentry)
{
	return dentry->d_name.name != dentry->d_iname;
}

This could possibly be made a little more clear....

NeilBrown
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/