All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andreas Gruenbacher <agruen@suse.de>
To: Neil Brown <neilb@suse.de>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Andrew Morton <akpm@osdl.org>, Tony Jones <tonyj@suse.de>
Subject: Re: [PATCH] Fix d_path for lazy unmounts
Date: Wed, 14 Feb 2007 18:43:49 -0800	[thread overview]
Message-ID: <200702141843.49668.agruen@suse.de> (raw)
In-Reply-To: <17862.30489.694079.138765@notabene.brown>

On Sunday 04 February 2007 16:15, Neil Brown wrote:
> The behaviour in the face of a lazy unmount should be clarified in
> this comment.

Done.

> If sys_getcwd is called on a directory that is no longer
> connected to the root, it isn't clear to me that it should return
> without an error.
> Without your patch it can return garbage which is clearly wrong.
> With you patch it will return a relative path name, which is also
> wrong (it isn't a valid path that leads to the current working directory).

Right, it should return -ENOENT instead in that case. Fixed as well.

> I would suggest that 'fail_deleted' be (e.g.) changed to
> 'fail_condition' where two conditions are defined
> #define DPATH_FAIL_DELETED 1
> #define DPATH_FAIL_DISCONNECTED 2

The much cleaner interface is to check if the path returned starts with a 
slash. If it doesn't, we know the path is bad as far as sys_getcwd() is 
concerned. We will construct the partial path in __d_path before figuring out 
that the path is disconnected, so no performance penalty, either.

> In reality, you are comparing "buflen < namelen+1" but spelling it as
> "buflen <= namelen".  I would prefer the full spelling with least room
> for confusion.

I'm fine either way.

> Maybe:
> > +		buflen -= namelen + 1;
> > +		buffer -= namelen + 1;
> > +		memcpy(buffer+1, dentry->d_name.name, namelen);
> > +		*buffer = '/';

That's better, yes.

Thanks,
Andreas

  reply	other threads:[~2007-02-15  2:43 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-03  3:23 [PATCH] Fix d_path for lazy unmounts Andreas Gruenbacher
2007-02-05  0:15 ` Neil Brown
2007-02-15  2:43   ` Andreas Gruenbacher [this message]
2007-02-05  8:32 ` Andreas Gruenbacher
2007-02-05 18:37   ` [PATCH]Add memory barrier before clear bit in unlock_buffer() Mingming Cao
2007-02-14  8:19   ` [PATCH] Fix d_path for lazy unmounts Andreas Gruenbacher
2007-02-14  8:29     ` Olaf Hering
2007-02-14  8:42       ` Andreas Gruenbacher
2007-02-14 15:37     ` Linus Torvalds
2007-02-14 19:39       ` Andreas Gruenbacher
2007-02-14 22:57         ` Andreas Gruenbacher
2007-02-15  3:13           ` Andreas Gruenbacher
2007-02-17 13:30             ` Andreas Gruenbacher
2007-02-15 12:53           ` Jan Engelhardt
2007-02-15 13:19             ` Andreas Gruenbacher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200702141843.49668.agruen@suse.de \
    --to=agruen@suse.de \
    --cc=akpm@osdl.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=tonyj@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.